Bug 32694 – WebKitLinkedOnOrAfter() ignores embedded frameworks

• Component: New Bugs
• Status: UNCONFIRMED

[Jeff Johnson]

Created attachment 45122 [details]
Sample Xcode project

Overview:
WebKit uses the function WebKitLinkedOnOrAfter(), defined in WebKit/mac/Misc/WebKitVersionChecks.m, to check the version of WebKit that an app is linked to. WebKitLinkedOnOrAfter() in turn calls NSVersionOfLinkTimeLibrary("WebKit"). However, NSVersionOfLinkTimeLibrary() only checks the main executable, and it returns -1 if the main executable did not link against the specified library. If you have an app that does not link to WebKit but that embeds a framework that does link to WebKit, NSVersionOfLinkTimeLibrary("WebKit") will return -1, but WebKit fails to check for that return value, and thus the value of WebKitLinkedOnOrAfter() has an unexpected result.

The unexpected result of WebKitLinkedOnOrAfter() can lead to undesired behavior. For example, I discovered this bug because the WebUIDelegate method webView:contextMenuItemsForElement:defaultMenuItems: wasn't getting called when right-clicking in a text field. This is because in the file WebKit/mac/WebCoreSupport/WebContextMenuClient.mm, there is a function isPreVersion3Client() with the following code:

    static BOOL preVersion3Client = !WebKitLinkedOnOrAfter(WEBKIT_FIRST_VERSION_WITH_3_0_CONTEXT_MENU_TAGS);

If the app does not link to WebKit but an embedded framework does, preVersion3Client gets the wrong value, YES. The result is that the wrong code is run in WebContextMenuClient::getCustomMenuFromDefaultItems(), and the WebUIDelegate method never gets called.

This bug is important because it is a common situation for embedded frameworks to contain WebKit code. For example, the popular software updater Sparkle run WebKit code. A lot of applications use Sparkle, but the applications themselves may not link to WebKit. Our suite of applications embed a framework that runs WebKit code for several purposes, such as showing a support contact form to the user. Again, the app itself may not link to WebKit.


Steps to reproduce:
1) Download, unzip, build, and run the attached sample Xcode project WebKitLinkBug.
2) Right-click in the "Last name" field.


Expected results:
The WebUIDelgate method webView:contextMenuItemsForElement:defaultMenuItems: gets called, returns an empty NSArray, and no contextual menu pops up.


Actual results:
The WebUIDelgate method webView:contextMenuItemsForElement:defaultMenuItems: doesn't get called, and a contextual menu containing default items pops up.


Regression:
This bug occurs on Max OS X 10.6.2 and Mac OS X 10.5.8. It also occurs with WebKit svn revision 51668 built from source.


Notes:
You can see in the console log when webView:contextMenuItemsForElement:defaultMenuItems: gets called. You can also see that NSVersionOfLinkTimeLibrary("WebKit") returns -1. However, if you go back into the WebKitLinkBug project, and link the WebKitLinkBug target to WebKit, then the bug does not occur, and NSVersionOfLinkTimeLibrary("WebKit") returns 34801920.

[Mark Rowe (bdash)]

I don’t think it’s possible to make these sorts of linked-on-or-after checks work correctly in the general case.  WebKit has no information about which particular piece of code (whether it be application, framework or plug-in) happens to be using it at the particular moment.  The application may have been written against a newer version of WebKit than a framework that it uses.  An application plug-in may have been written against an older version of WebKit than the application that loads it.

It seems like what you’re really asking for is an explicit way to opt in to the new behavior of one or more of these checks.  One approach that could avoid the limitations of checking linkage information would be to add a method to the relevant WebKit delegate that the client could implement to say “Yes, I really am ok with having the new behavior for the WebView’s that I’m the delegate for”.  A delegate that didn’t implement this new method would be determined by the linked-on-or-after check as per the current behavior.

[Mark Rowe (bdash)]

<rdar://problem/7483619>

[Mark Rowe (bdash)]

It’s unclear how we’d handle the smaller issue of WebKit being used by a framework from within an application that itself doesn’t link against WebKit.  Should we assume it support the new behavior or should we be safe and give it the old?  We’re effectively doing the latter at present.

[Jeff Johnson]

I believe that something needs to be done, because it's just going to become a bigger problem over time. You could ship WebKit version 999.9.9, and I could build a framework against the Mac OS X 10.9 SDK, and WebKit will still think that the linked version is -1.

Moreover, it seems pretty clear that this was an unanticipated bug. I've discovered a comment in the file WebKitVersionChecks.h: "A version of -1 is returned if the main executable did not link against WebKit (should never happen)." It should never happen, but in fact it does happen all the time, in shipping applications. That comment is like an assertion without the assert(). ;-)

I'm not sure to what extent I'd want explicit opt-in methods. These version checks are all WebKit-internal backward-compatibility hacks. As a client of the WebKit framework, I shouldn't need to know the implementation details. I simply want my code to work as expected: e.g., if I set the WebUIDelegate, then I expect the webView:contextMenuItemsForElement:defaultMenuItems: callback.

There's no problem with the application-specific, bundle identifier based checks (as long as the app developer is aware of them). But if it's not possible for the linked version checks to work correctly in the general case, then that kind of check should be avoided as much as possible. Don't change the behavior unless you're going to change the API. API-based checks are far superior. If an object still implements a deprecated method, then the old behavior must be assumed, and if an object implements a method that was newly introduced in a certain WebKit version, then new behavior can be assumed. I have no objection to 'opting-in' by implementing new API methods, but philosophically I'd be opposed to opt-in methods that are specifically tailored for embedded frameworks to implement in order to work around specific internal  backward-compatibility hacks.

[Jens Ayton]

I’m confused as to why you’d make a test like that before calling a delegate method in the first place.

My expectation as a Cocoa programmer is that the newest form of delegate method will be called if available, otherwise an older form will be used. AppKit and Foundation have linkage checks, but never, as far as I’m aware, for delegate methods.