5.22 KB, application/octet-stream
2.40 KB, text/plain
4.55 KB, patch
|Details | Diff | Splinter Review|
IMVU's installable client is based on embedded XULRunner, but the Mac App Store rejected us because XULRunner uses two private APIs: http://mxr.mozilla.org/mozilla1.9.1/search?string=_NSGetCarbonMenu http://mxr.mozilla.org/mozilla1.9.1/search?string=_LSCopyDefaultSchemeHandlerURL We're currently running 1.9.1, blocked on upgrading to 1.9.2 due to https://bugzilla.mozilla.org/show_bug.cgi?id=533245. Either way, it looks like 1.9.2 also uses these APIs. What would it take to remove these API calls from XULRunner so the Mac App Store would accept us?
I am interested to know if anyone has any ideas about this?
Created attachment 576473 [details] [diff] [review] patch with fix I've made a patch which fixes this issue. I don't know why the original author of the file suggests that LSGetApplicationForURL will fail. In my tests it works all them time. Can someone review the code and merge with central if approved? If the worry is that this fix wont be appropriate, then can we add build option for mac app store compatibility? i.e. ac_add_options --mac-app-store or something like this to turn on the feature. Thanks
Created attachment 576475 [details] sample test application This sample xul application tests the patch. As you can see it works well for all default url handlers.
You should probably get rid of the declaration of _LSCopyDefaultSchemeHandlerURL as well.
Comment on attachment 576475 [details] sample test application > You should probably get rid of the declaration of > _LSCopyDefaultSchemeHandlerURL as well. Please attach a new version of your patch that does this. I'll r+ it and land it on mozilla-inbound (from which it will quickly find its way onto mozilla-central). > the Mac App Store rejected us because XULRunner uses two private > APIs: > > http://mxr.mozilla.org/mozilla1.9.1/search?string=_NSGetCarbonMenu > http://mxr.mozilla.org/mozilla1.9.1/search?string=_LSCopyDefaultSchemeHandlerURL The Mozilla tree uses many more private APIs than these two. It will be impossible to remove them all. Apple's own apps use private APIs (I'll have more to say about this in a later comment). So must any app above a certain level of complexity. It's hypocritical for Apple to reject apps from the App Store that use private APIs, and pointless for Mozilla to try to meet this demand in full. Nonetheless, you seem to have found us using one private API (_LSCopyDefaultSchemeHandlerURL()) that we no longer need to use. The public API with which you replace it (LSGetApplicationForURL()) is used elsewhere in the tree (in nsMacShellService.cpp), apparently without any problems. And in my (brief) tests of your patch, I didn't see any problems, either. _NSGetCarbonMenu is no longer used as of FF 4.0. The 1.9.1 branch is no longer maintained. There's no chance that we'll backport this bug's patch to the 1.9.2 branch (the FF 3.6.X branch).
I tracked our first use of _LSCopyDefaultSchemeHandler() back to 2005, in the patch for bug 264648 (see particularly bug 264648 comment #5 and bug 264648 comment #7). Neither the bug nor the code comments say much about LSGetApplicationForURL()'s failures. I'm not a bit surprised that there *were* failures -- Launch Services is complex, and I've seen quite a few myself (with different methods). But whatever they were, they seem to have disappeared a while ago. Nonetheless we'll need to keep our eyes open for problems that may be caused by no longer using _LSCopyDefaultSchemeHandler().
As I mentioned above in comment #5, Apple's own apps use private APIs. I've confirmed this with Safari. But I'm quite sure other Apple apps also do so. It's quite easy to find out. Here's how to do it: Find an application binary (usually in the app bundle's Contents/MacOS/ directory) and run 'ns -pam' on it. This will print all of the binary's symbols to stdout. Among them will be many symbols imported from other binaries, some of which will be system libraries or frameworks. All of their names will be prefixed with an extra underscore character. Here are two examples, from the output for Safari: (undefined [lazy bound]) external _abort (from libSystem) (undefined [lazy bound]) external __LSCopyDefaultSchemeHandlerURL (from CoreServices) 'abort' is (of course) a standard POSIX call, and well documented. '_LSCopyDefaultSchemeHandlerURL' is, as we already know, not documented :-) Note that Safari's use of private APIs is doubly private: The Safari binary itself has almost no public symbols. But it links to the Safari framework in /System/Library/PrivateFrameworks. It's from there that I got the two symbols listed above.
Created attachment 578147 [details] Stack showing Safari directly calling _LSCopyDefaultSchemeHandlerURL (Following up comment #7) To further confirm what I said about Safari using private APIs, here's a stack of _LSCopyDefaultSchemeHandlerURL being called directly from code in the Safari framework (specifically from the dictionaryForScheme: method of the DefaultWebAppPopUpController class). Note that _LSCopyDefaultSchemeHandlerURL is not being called from some other, documented Launch Services API. To get this stack, I ran Safari in gdb, set a breakpoint on _LSCopyDefaultSchemeHandlerURL, and chose Safari : Preferences.
> 'ns -pam' Oops, sorry. This should have been 'nm -pam'.
Steven, I didn't get a sense of whether your comments indicated you were in favour of this patch or not. Could you make a call one way or the other instead of this limbo?
(In reply to comment #10) See comment #5, particularly the following: > Please attach a new version of your patch that does this. I'll r+ it > and land it on mozilla-inbound (from which it will quickly find its > way onto mozilla-central). As I've explained above, it's a fool's errand to stop using private APIs altogther. Apple knows this as well as I do, judging from it's own (i.e. Safari's) use of private APIs. But, as I already stated quite clearly in comment #5, I'm happy to stop using particular private APIs if there are reasonable non-private alternatives. I've been waiting all this time for someone to follow my recommendation, or to give some reason for not following it.
> But, as I already stated quite clearly in comment #5, I'm happy to > stop using particular private APIs if there are reasonable > non-private alternatives. Even if (as in this case) Safari continues to use the very private API that we'd no longer be using.
I think that we should aim to make developers' lives easier. If apple is not approving private api than perhaps we should limit their use. There are two reasons not to use private apis: * Imagine the benefit you will get if more xul-based applications appear on the appstore. I am sure that this will increase the number of people messing with the mozilla platform and contributing bug fixes, etc. I am not one of the most avid contributors since I just work for myself and I don't have a lot of time on my hands but nevertheless I try to help out where I can spare resources. This is all due to the fact that I can use xulrunner for my job. IMHO the more people there are out there like me the better because we do resilience testing of mozilla's code under many different constraints and when we find a bug we either fix it, report it or lobby to get it fixed. * Private apis are subject to change. Tomorrow Apple may decide to release an update which removes some of these APIs or limit their use. That will require mozilla to release an update for all mac os x users which will turn into disaster because the update will fail due to ff wont be able to launch at first place. Apple is certainly moving towards the iOS way with lion so this is more than anticipated at the moment. Now, if there are situations where it is simply not possible to use anything else but private APIs then fair enough. As far as I know my patched version (patch above) of xulrunner passes the appstore approval process. Perhaps I am not hitting the rest of the private APIs and therefore are not detected... it may as well be a security bug with appstore (I work in security, so I will not discuss it here) who knows. What I want to get through is that at least the patch which I have provided (with minor corrections, we need to remove the external definition) works and eliminates one instance of private api usage which we can go without.
(In reply to comment #13) You've given the standard argument against using private APIs, and it's entirely beside the point. As I've already said above, there are two main reasons for this: 1) Any app above a certain level of complexity needs to use private APIs. This includes Firefox and Safari. I know this to be true from personal experience on both OS X and Windows. 2) Apple's own apps and frameworks use private APIs on a major scale. So it's hypocritical of Apple to attempt to make other vendors' apps not use private APIs. Now if Apple started a program to solicit requests from other vendors to make public certain private APIs, and was willing to approve all reasonable requests, the situation would be entirely different. But this certainly hasn't been how Apple has behaved up til now. I'm not suggesting that apps should use just any private API. There should be no decent public alternative. And it should be APIs that the OS vendors own apps use, and which ideally haven't changed for the last several major versions of the OS. I know from my own experience that private APIs used by Apple's own apps *don't* change in minor updates -- for the very good reason that this would inconvenience Apple's own developers. They sometimes do change in major updates, and one needs to watch out for that. But I don't know of a single case where this has happened with a private API used by Firefox.
(In further reply to comment #13) As I've already said, more than once, I'm happy to r+ your patch if you revise it according to my suggestion (actually Josh Matthews' suggestion) from comment #5. Then I can land it in the tree, and it will appear in trunk nightlies in the next few days. I'm also willing to listen to suggestions that we stop using other private APIs. But all of these private APIs were originally used for a reason, and that reason may still be valid. I don't think the current situation justifies a major effort (on my part on anyone else's) to identify *all* the private APIs we use, and search for replacements for them. Yes, you may think that you've stepped in a hornet's nest. I'm sorry for that. But it's there for a good reason :-) I've no idea why Apple's App Store approval process identified only those private APIs you mentioned in comment #0. There could be any number of reasons.
Attachment #576473 - Flags: review?(smichaud)
Created attachment 596763 [details] [diff] [review] Passfree's patch with Josh Matthews' suggested change I got tired of waiting.
Comment on attachment 596763 [details] [diff] [review] Passfree's patch with Josh Matthews' suggested change Landed on mozilla-inbound: http://hg.mozilla.org/integration/mozilla-inbound/rev/27099c5bfca1
Assignee: nobody → smichaud
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
The patch as committed leaves behind the comments explaining why _LSCopyDefaultSchemeHandlerURL is being used, which is misleading given that it's not being used anymore…
In reply to comment #20) Where?
Sorry, never mind, it looks like that comment is referring to NSURLFileTypeMappingsInternal, not to _LSCopyDefaultSchemeHandlerURL.
Any change this change could be backported to the 10.0 branch, so any future ESR releases will have it?
You need to log in before you can comment on or make changes to this bug.