Mike Taylor [:miketaylr] Assignee
	Description • 9 years ago

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.

	Mike Taylor [:miketaylr] 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).

	Mike Taylor [:miketaylr] 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

	Mike Taylor [:miketaylr] Assignee
	Comment 5 • 9 years ago

(I probably should have split that commit into two, happy to do so if others agree).

	Kevin Brosnan [Ex-Mozilla]
	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?

	Mike Taylor [:miketaylr] 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.

	Hallvord R. M. Steen [:hallvors]
	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;
};

	Mark Finkle (:mfinkle) (use needinfo?)
	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?

	Richard Newman [:rnewman]
	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.

	Brian Birtles (:birtles, Feb 26~ parental leave)
	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?

	Karl Dubost💡 :karlcow
	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. ^_^

	Brian Birtles (:birtles, Feb 26~ parental leave)
	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?

	Karl Dubost💡 :karlcow
	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.

	Brian Birtles (:birtles, Feb 26~ parental leave)
	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.

	Karl Dubost💡 :karlcow
	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.)

	Mike Taylor [:miketaylr] 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.

	Mike Taylor [:miketaylr] 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)

	Mike Taylor [:miketaylr] Assignee
	Comment 19 • 9 years ago

Changed ANDROID -> MOZ_WIDGET_ANDROID and fixed the accidental 4.0 change.

	Mike Taylor [:miketaylr] 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.

	Richard Newman [:rnewman]
	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!

	Gervase Markham [:gerv]
	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

	Mike Taylor [:miketaylr] 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.

	Mike Taylor [:miketaylr] 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.

	Mike Taylor [:miketaylr] 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.

	Gervase Markham [:gerv]
	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

	Mike Taylor [:miketaylr] 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

	Mike Taylor [:miketaylr] 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).

	Mike Taylor [:miketaylr] 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.

	Pulsebot
	Comment 30 • 9 years ago

https://hg.mozilla.org/integration/fx-team/rev/b569676e397e

	Ryan VanderMeulen [:RyanVM]
	Comment 31 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/b569676e397e

	Mike Taylor [:miketaylr] 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

	Kohei Yoshino [:kohei]
	Comment 33 • 9 years ago

https://developer.mozilla.org/en-US/Firefox/Releases/41/Site_Compatibility#Networking

	Mike Hommey [:glandium]
	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)

	Niels Leenheer (HTML5test)
	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.

	Mike Taylor [:miketaylr] 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.

	Catalin Suciu [:csuciu]
	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

	hexalys
	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.