Bug 12854 – add a way to override navigator.appVersion separately from user agent

• Component: WebCore JavaScript
• Status: RESOLVED INVALID

[Oscar Cwajbaum]

Currently, navigator.appVersion is set to everything after the slash in the userAgent. Normally, this is sufficient, but there are two circumstances where  it is desirable to change this behavior:

* If the client wants a FireFox-style appVersion. Unlike IE, FireFox does not derive its appVersion from its userAgent.

* If the client wants to send a different appVersion for certain web sites for compatibility reasons. I am aware of at least one commercial browser that has an appVersion "exceptions" list for sites that insist on a particular appVersion.

The attached patch allows FrameLoaderClient to set the appVersion in the same way it currently sets userAgent.

To avoid breaking existing FrameLoaderClient implementations, I added the existing logic for deriving appVersion from the FrameLoaderClient class, so FrameLoaderClient subclasses that don't override the navigatorAppVersion method will get the current behavior. If it would be preferable to keep FrameLoaderClient pure virtual and update all FrameLoaderClient subclasses or if there is some better way to avoid breaking existing subclasses, I'll update the patch.

[Oscar Cwajbaum]

Created attachment 13313 [details]
patch: Allow FrameLoaderClient to override navigator.appVersion

[Darin Adler]

Comment on attachment 13313 [details]
patch: Allow FrameLoaderClient to override navigator.appVersion

Issues like this one, which affect compatibility with websites, for the most part should be consistent across multiple versions of the WebKit engine. We should not have different approaches to site compatibility for different versions of WebKit unless there's a compelling reason to do so.

If reason (1) is truly compelling, then I suggest having the user agent string change, and not just the appVersion. Early versions of WebKit did indeed use a different user agent on different websites, but after much reflection we decided this was not a scalable approach to site compatibility.

As far as reason (2), if a browser wants to spoof and report different things to different websites, I recommend changing the user agent string rather than just changing appVersion.

Overall, I think that the way this patch is coded is fine, but I'm not convinced this is a good feature to add to WebKit.

Marking the local variable "ua" as const isn't really all that helpful, especially in a one-line, two-statement function, so I suggest leaving it off.

I'd find this more convincing if someone said specifically that they wanted to use this in a particular version of WebKit and explained the benefits in a more concrete way.

[Darin Adler]

(In reply to comment #2)
> If reason (1) is truly compelling, then I suggest having the user agent string
> change, and not just the appVersion. Early versions of WebKit did indeed use a
> different user agent on different websites, but after much reflection we
> decided this was not a scalable approach to site compatibility.
> 
> As far as reason (2), if a browser wants to spoof and report different things
> to different websites, I recommend changing the user agent string rather than
> just changing appVersion.

Oops, I got this wrong. Those are both arguments about "different appVersion for certain web sites".

What I meant to say about the Firefox-style appVersion is that for most of these compatibility-affecting features we want a standard approach in WebKit, and not a diversity of approaches in the various WebKit-using browsers.

That's because as much as possible we want to all be in "the same boat" for compatibility. If there is indeed a compelling reason to match Firefox instead of IE in this property, I'd like to see it applied to all versions of WebKit, not to one particular WebKit-based browser.

[Oscar Cwajbaum]

> If reason (1) is truly compelling, then I suggest having the user agent string
> change, and not just the appVersion. Early versions of WebKit did indeed use a
> different user agent on different websites, but after much reflection we
> decided this was not a scalable approach to site compatibility.
> As far as reason (2), if a browser wants to spoof and report different things
> to different websites, I recommend changing the user agent string rather than
> just changing appVersion.

Since the userAgent is already set by the client, any per-site compatibility code can already modify the userAgent. For instance, if a particular site uses ActiveX unless the userAgent and appVersion are FireFox, the client can modify the userAgent but not the appVersion. This patch allows the client to modify both if it so desires. Why tie the two together?

> I'd find this more convincing if someone said specifically that they wanted to
> use this in a particular version of WebKit and explained the benefits in a more
> concrete way.

For example, if you look at the browser detection code on eonline.com, it has several problems. Last I tried it, it choked when it detected WebKit on Linux. Also, it checks for specific strings. Since have a WebKit-based browser returning "webkit safari" might be considered a trademark violation, the next best thing is to masquerade as FireFox. 

> That's because as much as possible we want to all be in "the same boat" for
> compatibility. 

The client can already set appVersion indirectly (by modifying the UA), so depending on the UA that is set, we all are already not in the same boat. Why allow any UA string, but then force the appVersion to be a substring?

Also, what if the chosen UA string doesn't have a slash in it?

> Marking the local variable "ua" as const isn't really all that helpful,
> especially in a one-line, two-statement function, so I suggest leaving it off.

OK, thanks. If this change is OK'd I'll submit a modified patch.

[Oscar Cwajbaum]

Created attachment 13338 [details]
Updated patch

I removed 'const' as suggested.

Please reconsider this patch for the reasons I stated earlier. In summary, client code can already affect appVersion, and therefore introduce compatibility differences. Why restrict the appVersion to a substring of the UA, especially when there are cases that this is not desirable?

[Darin Adler]

(In reply to comment #4)
> For example, if you look at the browser detection code on eonline.com, it has
> several problems. Last I tried it, it choked when it detected WebKit on Linux.
> Also, it checks for specific strings.

Lets dig into this eonline.com case.

First of all, it's entirely a user agent check, so not at all relevant to a discussion of separately changing appVersion to be different from the user agent. The site doesn't look at appVersion at all.

The site's check is for "applewebkit/", which is the correct way to identify WebKit.

I don't think the way to fix this is to have a version of WebKit that leaves WebKit out of the user agent string!

It's unfortunate that eonline.com fails when the user agent string contains the substring "linux". You should try to get the site fixed, and also, if you really want to spoof to make the site work, I suggest a user agent string that doesn't contain "linux" or "x11" in it.

> Since have a WebKit-based browser returning "webkit safari" might be considered a trademark violation

That's a straw man. I agree that a non-Safari browser should not include the string Safari in its user agent string. But the string you cite, "webkit safari", is part of the eonline.com website, not a string supplied by WebKit.

This is *not* a trademark violation.

> the next best thing is to masquerade as FireFox.

1) That's a really bad strategy for the long term. We want sites to check for WebKit. We don't want some versions of WebKit to masquerade as Firefox in a way that's different from other versions of WebKit. Long term, these kinds of differences between different versions of WebKit are bad for all of us using WebKit.

2) This has nothing to do with appVersion.

I think this patch is a bad idea.

[Darin Adler]

Comment on attachment 13338 [details]
Updated patch

(In reply to comment #5)
> Why restrict the appVersion to a substring of the
> UA, especially when there are cases that this is not desirable?

Please cite a case where this is not desirable so we can discuss it.

Until then, review-.

[Andrew Wellington]

Please re-open this if you wish to cite a case where this is desirable as requested by Darin in Comment #7. Until then, I think we should close this bug.