Closed
Bug 1169772
Opened 9 years ago
Closed 9 years ago
Add Android version number to Fennec UA String
Categories
(Firefox for Android Graveyard :: General, defect)
Firefox for Android Graveyard
General
Tracking
(p11+, firefox41 verified, fennec41+)
VERIFIED
FIXED
Firefox 41
People
(Reporter: miketaylr, Assigned: miketaylr)
References
Details
(Keywords: dev-doc-complete, site-compat, Whiteboard: [compat])
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
miketaylr
:
review+
|
Details | Diff | Splinter Review |
To be more compatible with the mobile web, we should add the Android version number to our UA string:
Mozilla/5.0 (Android <version number>; Mobile; rv: <gecko version>) Gecko/<gecko version> Firefox/<gecko version>.
We'll also want to write a blog post, update MDN, and let some of our device detection friends know about this change.
Assignee | ||
Comment 1•9 years ago
|
||
To be able to quickly fix any regressions we do find, we should wait until we have the dynamic UA override CDN stuff is in place (Bug 1163759).
Updated•9 years ago
|
Whiteboard: [compat]
Assignee | ||
Comment 3•9 years ago
|
||
Here's some data that John Jensen helped us with.
The UAs he tested are (0): Windows Phone, (1): Chrome Mobile, (2): Current Fennec, and (3): Fennec + Android version
The pair we're interested in is (1, 3)
Num UA
0 Mozilla/5.0 (Windows Phone 8.1; ARM; Trident/7.0; Touch; rv:11;
IEMobile/11.0) like Android 4.1.2; compatible) like iPhone OS 7_0_3
Mac OS X WebKit/537.36 (KHTML, like Gecko) Chrome/32.0.1700.99 Mobile
Safari/537.36
1 Mozilla/5.0 (Linux; Android 4.4.4; Galaxy Nexus Build/JRN84D)
AppleWebKit/537.36 (KHTML, like Gecko) Chrome/42.0.2311.111.166 Mobile
Safari/537.36
2 Mozilla/5.0 (Android Mobile; rv:40.0) Gecko/40.0 Firefox/40.0
3 Mozilla/5.0 (Android 4.4.4; Mobile; rv:40.0) Gecko/40.0 Firefox/40.0
Top 10
Pair N Min Max Median Mean StDev CoV
(0, 1) 10 0.289 1.000 0.975 0.843 0.266 0.316
(0, 2) 10 0.000 1.000 0.989 0.887 0.296 0.334
(0, 3) 10 0.000 1.000 0.987 0.887 0.296 0.334
(1, 2) 10 0.001 1.000 0.990 0.757 0.367 0.485
(1, 3) 10 0.001 1.000 0.991 0.759 0.368 0.485
(2, 3) 10 0.972 1.000 0.999 0.992 0.010 0.010
Top 100
Pair N Min Max Median Mean StDev CoV
(0, 1) 100 0.195 1.000 0.987 0.809 0.289 0.357
(0, 2) 100 0.000 1.000 0.997 0.925 0.196 0.212
(0, 3) 100 0.000 1.000 0.997 0.934 0.190 0.203
(1, 2) 100 0.001 1.000 0.993 0.809 0.292 0.361
(1, 3) 100 0.001 1.000 0.994 0.820 0.289 0.353
(2, 3) 100 0.372 1.000 1.000 0.979 0.080 0.082
Top 1,000
Pair N Min Max Median Mean StDev CoV
(0, 1) 998 0.020 1.000 1.000 0.926 0.204 0.220
(0, 2) 999 0.000 1.000 1.000 0.946 0.171 0.181
(0, 3) 998 0.000 1.000 1.000 0.966 0.135 0.140
(1, 2) 998 0.001 1.000 1.000 0.908 0.221 0.243
(1, 3) 999 0.001 1.000 1.000 0.934 0.192 0.206
(2, 3) 998 0.037 1.000 1.000 0.969 0.129 0.133
Top 10,000
Pair N Min Max Median Mean StDev CoV
(0, 1) 9963 0.000 1.000 1.000 0.974 0.122 0.125
(0, 2) 9962 0.000 1.000 1.000 0.971 0.126 0.130
(0, 3) 9961 0.000 1.000 1.000 0.979 0.108 0.110
(1, 2) 9960 0.000 1.000 1.000 0.969 0.132 0.137
(1, 3) 9961 0.000 1.000 1.000 0.978 0.112 0.114
(2, 3) 9966 0.002 1.000 1.000 0.986 0.084 0.086
All
Pair N Min Max Median Mean StDev CoV
(0, 1) 87670 0.000 1.000 1.000 0.987 0.088 0.089
(0, 2) 87668 0.000 1.000 1.000 0.983 0.098 0.100
(0, 3) 87667 0.000 1.000 1.000 0.986 0.089 0.090
(1, 2) 87677 0.000 1.000 1.000 0.986 0.090 0.091
(1, 3) 87678 0.000 1.000 1.000 0.990 0.076 0.077
(2, 3) 87680 0.002 1.000 1.000 0.992 0.065 0.065
Assignee | ||
Comment 4•9 years ago
|
||
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → miket
Assignee | ||
Comment 5•9 years ago
|
||
(I probably should have split that commit into two, happy to do so if others agree).
Comment 6•9 years ago
|
||
Could we just lie here? Hard code it to a modern version of Android? What is the experience for users on Android 2.3 or 4.0 devices?
Assignee | ||
Comment 7•9 years ago
|
||
(In reply to Kevin Brosnan [:kbrosnan] from comment #6)
> Could we just lie here? Hard code it to a modern version of Android? What is
> the experience for users on Android 2.3 or 4.0 devices?
I thought about that. There are scripts in the wild that do things like,
if (androidVersion > 2.3) {
doSomethingCool()
} else {
doSomethingLame()
}
I don't know how widespread it is though. The upshot is typically these scripts will typically bomb because we don't match their regexp (which assumes a version number) and we'd end up with a broken page or in doSomethingLame for everyone.
One crazy option: if (version < 4) {pretendToBeVersion4()}
However, I'm sort of inclined to see what kind of fallout we'll actually run into for our 2.3 users and revisit.
Comment 8•9 years ago
|
||
FYI, some Brightcove code below.. note that at least the first method here must return true for videos to show up in the page.
brightcove.player.utils.PlayerUtil.isAndroidWithInPagePlayback = function(pUserAgent) {
var
userAgent = pUserAgent || brightcove.userAgent,
isAmazonSilk = /Silk/g.test(userAgent),
androidVersion = (userAgent.match(/\bAndroid (\d+\.\d+)/) || [])[1];
return +androidVersion >= 3.1 && !isAmazonSilk;
};
brightcove.player.utils.PlayerUtil.isAndroidWithHLSSupport = function(pUserAgent) {
var
userAgent = pUserAgent || brightcove.userAgent,
androidVersion = (userAgent.match(/\bAndroid (\d+\.\d+)/) || [])[1];
return +androidVersion >= 4.0;
};
brightcove.player.utils.PlayerUtil.isAndroidWithAcceptableHLSSupport = function(pUserAgent) {
var
userAgent = pUserAgent || brightcove.userAgent,
androidChrome = (/android.*chrome/i).test(userAgent),
androidVersion = (userAgent.match(/\bAndroid (\d+\.\d+)/) || [])[1];
return androidChrome && +androidVersion >= 4.2;
};
Comment 9•9 years ago
|
||
Comment on attachment 8616265 [details] [diff] [review]
1169772-Add-Android-version-to-platform-identifi.patch
I am OK with adding the version number, especially since we know it will improve the display of web content.
One thing to check: ANDROID alone will be set for Fennec and B2G, IIRC. Is that the intention?
Attachment #8616265 -
Flags: review?(mark.finkle) → review+
Comment 10•9 years ago
|
||
> return +androidVersion >= 3.1 && !isAmazonSilk;
> };
Not only is this kind of conditional wrong for Fennec (because we ship the same rendering engine everywhere), but amusingly it's also wrong for Android distributions with the updatable WebView. Version sniffing fails again!
If we must include a version number, I would also be tentatively inclined to clamp the bottom end to, say, 4.1. It'll screw up people's stats, but that's their problem.
Comment 11•9 years ago
|
||
Comment on attachment 8616265 [details] [diff] [review]
1169772-Add-Android-version-to-platform-identifi.patch
>diff --git a/mobile/android/base/AppConstants.java.in b/mobile/android/base/AppConstants.java.in
>index bba2c28..2233978 100644
>--- a/mobile/android/base/AppConstants.java.in
>+++ b/mobile/android/base/AppConstants.java.in
>@@ -119,22 +119,24 @@ public class AppConstants {
> public static final String MOZ_UPDATE_CHANNEL = "@MOZ_UPDATE_CHANNEL@";
> public static final String OMNIJAR_NAME = "@OMNIJAR_NAME@";
> public static final String OS_TARGET = "@OS_TARGET@";
> public static final String TARGET_XPCOM_ABI = @TARGET_XPCOM_ABI@;
>
> public static final String USER_AGENT_BOT_LIKE = "Redirector/" + AppConstants.MOZ_APP_VERSION +
> " (Android; rv:" + AppConstants.MOZ_APP_VERSION + ")";
>
>- public static final String USER_AGENT_FENNEC_MOBILE = "Mozilla/5.0 (Android; Mobile; rv:" +
>+ public static final String USER_AGENT_FENNEC_MOBILE = "Mozilla/4.0 (Android " +
>+ Build.VERSION.RELEASE + "; Mobile; rv:" +
> AppConstants.MOZ_APP_VERSION + ") Gecko/" +
> AppConstants.MOZ_APP_VERSION + " Firefox/" +
> AppConstants.MOZ_APP_VERSION;
Sorry for the drive-by here, but why do we change the Mozilla version to 4.0?
Flags: needinfo?(miket)
Comment 12•9 years ago
|
||
Brian,
In short, for WebCompat reasons
In long, things like :)
* https://github.com/webcompat/web-bugs/issues/426#issuecomment-61869560
* https://github.com/webcompat/web-bugs/issues/1121#issuecomment-105809660
* https://github.com/webcompat/web-bugs/issues/936#issue-69720883
* https://github.com/webcompat/web-bugs/issues/1053#issue-73748486
* And many others
but it's still an interesting discussion to have if we can find a better way of doing it. ^_^
Comment 13•9 years ago
|
||
(In reply to Karl Dubost :karlcow from comment #12)
> Brian,
> In short, for WebCompat reasons
> In long, things like :)
>
> * https://github.com/webcompat/web-bugs/issues/426#issuecomment-61869560
> * https://github.com/webcompat/web-bugs/issues/1121#issuecomment-105809660
> * https://github.com/webcompat/web-bugs/issues/936#issue-69720883
> * https://github.com/webcompat/web-bugs/issues/1053#issue-73748486
> * And many others
>
> but it's still an interesting discussion to have if we can find a better way
> of doing it. ^_^
Those seem to be testing the Android version. Surely changing the *Mozilla* version introduces even more compat issues?
Comment 14•9 years ago
|
||
> Those seem to be testing the Android version.
Correct. And it's why we don't get candies when we fail this test.
> Surely changing the *Mozilla* version introduces even more compat issues?
Not so far, with preliminary tests, adding the version number fixes more issues than creating new ones (unfortunately). Believe me or ask Mike, I'm pushing to demonstrate it will create more issues, but I failed so far ;) It ranges to getting the nice CSS effects, the video library working only because of version numbers, the redirect to the right version of the site, JS with no fallback when Android version is false, etc. etc.
Comment 15•9 years ago
|
||
I understand how adding the Android version helps but I'm wondering how the change from "Mozilla/5.0" to "Mozilla/4.0" helps.
Comment 16•9 years ago
|
||
Aaaaaah… ok + IRC discussion with brian.
* No version number => bad for Web Compatibilityer
* version <= 2.3 => bad for Web Compatibility (in the case of Mobile Gecko)
* version = 4.0 => maximizing our chances
* version >= 5.0 => unknown but we could check if there are weird things going on or not
I have no strong opinions about it.
Just for the footnote, Most of the time Android means for people (Android == mobile == Blink). So when they put the version number it is often because a feature is thought to be available for this version of Android for Blink. (think audio, CSS, etc.)
Assignee | ||
Comment 17•9 years ago
|
||
(In reply to Brian Birtles (:birtles) from comment #11)
> >- public static final String USER_AGENT_FENNEC_MOBILE = "Mozilla/5.0 (Android; Mobile; rv:" +
> >+ public static final String USER_AGENT_FENNEC_MOBILE = "Mozilla/4.0 (Android " +
> >+ Build.VERSION.RELEASE + "; Mobile; rv:" +
> > AppConstants.MOZ_APP_VERSION + ") Gecko/" +
> > AppConstants.MOZ_APP_VERSION + " Firefox/" +
> > AppConstants.MOZ_APP_VERSION;
>
> Sorry for the drive-by here, but why do we change the Mozilla version to 4.0?
Hey Brian, thanks for the drive-by--that's actually not supposed to be in the patch. ^_^ I was just making a simple change to test that things were working. Forgot to set it back. Will fix.
Assignee | ||
Comment 18•9 years ago
|
||
(In reply to Mark Finkle (:mfinkle) from comment #9)
> Comment on attachment 8616265 [details] [diff] [review]
> One thing to check: ANDROID alone will be set for Fennec and B2G, IIRC. Is
> that the intention?
Oh right. We don't want to add an Android version for B2G. I want MOZ_WIDGET_ANDROID rather than ANDROID.
(thx to kats for https://wiki.mozilla.org/index.php?title=Platform/Platform-specific_build_defines)
Assignee | ||
Comment 19•9 years ago
|
||
Changed ANDROID -> MOZ_WIDGET_ANDROID and fixed the accidental 4.0 change.
Attachment #8616265 -
Attachment is obsolete: true
Assignee | ||
Comment 20•9 years ago
|
||
(In reply to Hallvord R. M. Steen [:hallvors] from comment #8)
> FYI, some Brightcove code below.. note that at least the first method here
> must return true for videos to show up in the page.
>
> brightcove.player.utils.PlayerUtil.isAndroidWithInPagePlayback =
> function(pUserAgent) {
> var
> userAgent = pUserAgent || brightcove.userAgent,
> isAmazonSilk = /Silk/g.test(userAgent),
> androidVersion = (userAgent.match(/\bAndroid (\d+\.\d+)/) || [])[1];
> return +androidVersion >= 3.1 && !isAmazonSilk;
> };
Ugh. OK. Brightcove video is a big deal (and one of my main motivations for this patch).
(In reply to Richard Newman [:rnewman] from comment #10)
> If we must include a version number, I would also be tentatively inclined to
> clamp the bottom end to, say, 4.1. It'll screw up people's stats, but that's
> their problem.
Any reason for 4.1 over just 4.0? For the Brightcove case as long as we pass the "isAndroidWithInPagePlayback" test, we're good.
Comment 21•9 years ago
|
||
(In reply to Mike Taylor [:miketaylr] from comment #20)
> Any reason for 4.1 over just 4.0? For the Brightcove case as long as we pass
> the "isAndroidWithInPagePlayback" test, we're good.
JB 4.1 added a bunch of bidi text and locale support, and some audio stuff. I could certainly imagine sites sniffing for that.
The other major release to pin to would be 4.4, which is the point at which the system webview started tracking Chromium.
You're definitely the person with the data to make this decision, though!
Flags: needinfo?(rnewman)
Comment 22•9 years ago
|
||
I'm still not sure that adding a version number to the UA string that every Firefox for Android sends to every site in the world is the best way to solve the problems we are hitting. What lower-impact strategies have been considered? For Brightcove, for example, I'm told that the bad JS is normally served from a small number of CDN endpoints, so we should be able to add those to the UA spoofing list.
One of the reasons we didn't include the Android version number is that people would draw bogus inferences from it - an issue we can see a bit of in the comments above, and will see more of if we make this change. We will replace the problems we have from not having a version number, with new problems we have from not being able to send a version number that makes everyone happy. If we don't lie, we are really lying because people assume we are the 2.3 Android browser, which sucks. If we lie, then if the sites get it wrong, they are hardly to blame.
Gerv
Assignee | ||
Comment 23•9 years ago
|
||
(In reply to Gervase Markham [:gerv] from comment #22)
> I'm still not sure that adding a version number to the UA string that every
> Firefox for Android sends to every site in the world is the best way to
> solve the problems we are hitting. What lower-impact strategies have been
> considered? For Brightcove, for example, I'm told that the bad JS is
> normally served from a small number of CDN endpoints, so we should be able
> to add those to the UA spoofing list.
Strategies considered include:
Evangelism: This doesn't scale and is never as successful as we want. Even if we have the right contacts, often time for business reasons sites just can't go back and fix their Android version detection regexp's or update to fixed versions of libraries.
UA Overrides: This still plays a part of the equation. And we will probably need to use it to fix regressions that this patch might introduce.
But there's no way we can possibly fix the number of problems caused by a lack of Android version one site at a time. For example, check out this search result and poke around:
<https://github.com/search?l=javascript&p=6&q=android+%28[0-9]&ref=searchresults&type=Code&utf8=%E2%9C%93>
There are countless sites and deployed JS libraries that we can't reasonably expect to override.
"TypeError: /Android ([0-9\.]+)[\);]/.exec(...) is null" (or similar variation) is a rather large cause of many compat issues.
> One of the reasons we didn't include the Android version number is that
> people would draw bogus inferences from it - an issue we can see a bit of in
> the comments above, and will see more of if we make this change.
Probably yes. I'm not convinced that we will be breaking more sites than we will be fixing, based on the experience that our team has triaging Fennec compat bugs for the past few years.
> We will
> replace the problems we have from not having a version number, with new
> problems we have from not being able to send a version number that makes
> everyone happy. If we don't lie, we are really lying because people assume
> we are the 2.3 Android browser, which sucks.
We know that not having an Android version number in the Fennec UA string breaks many sites.
For some more context, in a list of top 126 mobile sites in Japan, 19 of the broken ones are broken because they're missing an Android version number. (we don't end up on a mobile site, can't play video, or can't swipe carousels etc.). With this patch, those 19 are fixed and 2 properties break.
I think it makes more sense to ship UA overrides for 2 sites than 19.
Also, one aspect of the "lying about the version number" problem will go away once everyone is off of Android devices < 4ish, or we stop supporting that version (maybe in 5 years, just a wild guess).
> If we lie, then if the sites get it wrong, they are hardly to blame.
Sure. And our team is going to be maintaining the UA override list to take care of these sites until they can get fixed. But until we find the sites that will break with this change we can't know for sure. Riding the trains would be helpful here, I think.
Assignee | ||
Comment 24•9 years ago
|
||
(In reply to Richard Newman [:rnewman] from comment #21)
> (In reply to Mike Taylor [:miketaylr] from comment #20)
>
> > Any reason for 4.1 over just 4.0? For the Brightcove case as long as we pass
> > the "isAndroidWithInPagePlayback" test, we're good.
> The other major release to pin to would be 4.4, which is the point at which
> the system webview started tracking Chromium.
>
> You're definitely the person with the data to make this decision, though!
Ah, gotcha. I think pretending to be KitKat (4.4) is probably the best bet here. Might as well be the most up-to-date of the last major version.
Assignee | ||
Comment 25•9 years ago
|
||
This version of the patch will pretend to be 4.4 if it's not already running on a 4+ Android.
Assignee | ||
Updated•9 years ago
|
Attachment #8616741 -
Attachment is obsolete: true
Comment 26•9 years ago
|
||
Comment on attachment 8616969 [details] [diff] [review]
1169772-Add-Android-version-to-platform-identifi-3.patch
Review of attachment 8616969 [details] [diff] [review]:
-----------------------------------------------------------------
r=gerv. <sigh>
Gerv
Attachment #8616969 -
Flags: review?(gerv) → review+
Assignee | ||
Comment 27•9 years ago
|
||
Thanks Gerv.
Here's an updated patch to reflect r=mfinkle, gerv.
(I left out the sigh from the commit message.)
And here's a link to try to see if anything blows up: https://treeherder.mozilla.org/#/jobs?repo=try&revision=be83d34da06b
Attachment #8616969 -
Attachment is obsolete: true
Assignee | ||
Comment 28•9 years ago
|
||
Reminder to self: we can't land this until 1163759 is ready. I'll also send out an email to all our device-detection framework friends as a courtesy heads up (I've tested the major ones and nothing breaks).
Keywords: dev-doc-needed
Updated•9 years ago
|
tracking-fennec: ? → 41+
Assignee | ||
Comment 29•9 years ago
|
||
Bug 1175305 is on fx-team, so setting this to checkin-needed. The try run in Comment #27 looks good and I've just rebased to make sure there's no bit-rot.
Keywords: checkin-needed
Comment 30•9 years ago
|
||
Keywords: checkin-needed
Comment 31•9 years ago
|
||
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Assignee | ||
Comment 32•9 years ago
|
||
I updated the MDN docs: https://developer.mozilla.org/en-US/docs/Web/HTTP/Gecko_user_agent_string_reference#Android_%28version_41_and_above%29
Keywords: dev-doc-needed
Comment 33•9 years ago
|
||
Keywords: dev-doc-needed,
site-compat
Comment 34•9 years ago
|
||
Why not just pretend to be Android 4.4 for *all* versions of Android? (and possibly bump to 5.x later)
Comment 35•9 years ago
|
||
If you must lie about version numbers, I'd rather see Firefox lie in an obvious way. For example instead of using 4.4, I'd suggest to use "4.0-" or "4.5" even "4.4.99". I think all of these examples would satisfy the reason why this change was made and would still allow version detection for people who know what they are doing.
And if you don't want version detection at all, just use 4.4 for every version and tell device detection services to ignore it.
Assignee | ||
Comment 36•9 years ago
|
||
> Why not just pretend to be Android 4.4 for *all* versions of Android? (and
> possibly bump to 5.x later)
It would require bumping later, without fail there will be sites that require version 5 and up (if they don't already exist).
(In reply to Niels Leenheer (HTML5test) from comment #35)
> If you must lie about version numbers, I'd rather see Firefox lie in an
> obvious way. For example instead of using 4.4, I'd suggest to use "4.0-" or
> "4.5" even "4.4.99". I think all of these examples would satisfy the reason
> why this change was made and would still allow version detection for people
> who know what they are doing.
Personally I'd rather just wait until all our users are on KitKat and up and then we don't have to lie about anything.
Comment 37•9 years ago
|
||
Tested on several devices on latest Nightly and Aurora on www.whatsmyua.com
On devices with Android < 4.0 (2.3.x, 3.x), the Android version number is 4.4
On devices with Android >= 4.0, Fennec UA String displays the Android version from device
Verifying as fixed
Status: RESOLVED → VERIFIED
Comment 38•9 years ago
|
||
That's a(In reply to Mike Taylor [:miketaylr] from comment #36)
> > Why not just pretend to be Android 4.4 for *all* versions of Android? (and
> > possibly bump to 5.x later)
>
> It would require bumping later, without fail there will be sites that
> require version 5 and up (if they don't already exist).
>
> (In reply to Niels Leenheer (HTML5test) from comment #35)
> > If you must lie about version numbers, I'd rather see Firefox lie in an
> > obvious way. For example instead of using 4.4, I'd suggest to use "4.0-" or
> > "4.5" even "4.4.99". I think all of these examples would satisfy the reason
> > why this change was made and would still allow version detection for people
> > who know what they are doing.
>
> Personally I'd rather just wait until all our users are on KitKat and up and
> then we don't have to lie about anything.
The 4.0- format sounds weird and isn't used anywhere. But I agree that lying isn't very good here.
If need change or you think it's better, I would suggest 3+ or 2.3+ as the preferred approach.
The [+] practice is already used as flags by 2-3 webkit engine forks. This helps indicating the minimum possible implied support, as opposed to a version that is statistically trustable.
If inferences are made on that number, it's better *not* to imply a feature that may not supported.
Updated•9 years ago
|
Keywords: dev-doc-needed → dev-doc-complete
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•