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)

defect
Not set
normal

Tracking

(p11+, firefox41 verified, fennec41+)

VERIFIED FIXED
Firefox 41
Tracking Status
p11 + ---
firefox41 --- verified
fennec 41+ ---

People

(Reporter: miketaylr, Assigned: miketaylr)

References

Details

(Keywords: dev-doc-complete, site-compat, Whiteboard: [compat])

Attachments

(1 file, 3 obsolete files)

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.
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).
tracking-fennec: --- → ?
tracking-p11: --- → ?
Depends on: 1163759
Whiteboard: [compat]
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: nobody → miket
(I probably should have split that commit into two, happy to do so if others agree).
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?
(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.
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 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+
> 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 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)
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. ^_^
(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?
> 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.
I understand how adding the Android version helps but I'm wondering how the change from "Mozilla/5.0" to "Mozilla/4.0" helps.
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.)
(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.
(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)
Changed ANDROID -> MOZ_WIDGET_ANDROID and fixed the accidental 4.0 change.
Attachment #8616265 - Attachment is obsolete: true
(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.
(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)
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
(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.
(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.
This version of the patch will pretend to be 4.4 if it's not already running on a 4+ Android.
Attachment #8616741 - Attachment is obsolete: true
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+
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
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
tracking-fennec: ? → 41+
Depends on: 1174784
Depends on: 1175305
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
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Depends on: 1178761
Why not just pretend to be Android 4.4 for *all* versions of Android? (and possibly bump to 5.x later)
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.
> 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.
Depends on: 1184320
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
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.
Depends on: 1193354
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: