philippec / PhFacebook

MacOSX Interface to Facebook graph API
http://developer.casgrain.com/?p=107
Other
178 stars 44 forks source link

there's a crtiical bug in this code #46

Closed johndpope closed 8 years ago

johndpope commented 9 years ago

I suggest you migrate it to use arc in xcode > edit > convert to objectice-c ARC

the synthesized variables can all be deleted. You shouldn't need any private variables here > as they were the same name - and I was seeing no respnse from the parent. I'd commit my code on fork but don't have time for thorough testing.

@interface PhWebViewController : NSObject { // delete these as the conflict PhFacebook *parent;

}

@property (unsafe_unretained) IBOutlet NSWindow window; @property (unsafe_unretained) IBOutlet WebView webView; @property (unsafe_unretained) IBOutlet NSButton cancelButton; @property (nonatomic,unsafe_unretained) PhFacebook parent; @property (nonatomic, strong) NSString *permissions;

philippec commented 9 years ago

Is there an actual bug or is conversion to ARC just a nice-to-have? Because manual retain-release still works, and doesn't require the new runtime which is why I haven't done it yet (I have an app that uses it that is old, supports old OSes).

I know how to convert to ARC. But in this case, PhFacebook is a separate framework so you can link it to an ARC-enabled app with no issues that I know of.

Unless there's an actual bug I see no reason to keep this request open with this title.

johndpope commented 9 years ago

I found the parent wasn't getting callback when targeting 10.10 It's flacky. Having the variables with same names as Ivars is dangerous. I was left scratching my head for hours troubleshooting. Even if you don't convert to arc - removing the internal variables with same name would resolve this issue.

JP

On Friday, June 19, 2015, Philippe Casgrain notifications@github.com wrote:

Is there an actual bug or is conversion to ARC just a nice-to-have? Because manual retain-release still works, and doesn't require the new runtime which is why I haven't done it yet (I have an app that uses it that is old, supports old OSes).

I know how to convert to ARC. But in this case, PhFacebook is a separate framework so you can link it to an ARC-enabled app with no issues that I know of.

Unless there's an actual bug I see no reason to keep this request open with this title.

— Reply to this email directly or view it on GitHub https://github.com/philippec/PhFacebook/issues/46#issuecomment-113351859 .

John Pope | Senior Software Engineer BG Mobile Apps Pty Ltd

Port Melbourne, Australia T +61423562387 | E jp@bellgeorge.com tom@bellgeorge.com IN http://www.linkedin.com/in/jdpope

philippec commented 9 years ago

I removed the naked ivar access which, for some reason, I only had done in one class. Try it now?

philippec commented 8 years ago

Having not heard back, closing the issue.