Last Comment Bug 671634 - Useragent should be different between phones and tablets
: Useragent should be different between phones and tablets
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: Networking: HTTP (show other bugs)
: Trunk
: All Other
: -- normal (vote)
: ---
Assigned To: Mark Finkle (:mfinkle) (use needinfo?)
:
Mentors:
: 734068 (view as bug list)
Depends on: 723746 729348 770112
Blocks: 723538 741094 588909 699289 700595 721632 809396
  Show dependency treegraph
 
Reported: 2011-07-14 12:48 PDT by Al
Modified: 2013-10-28 11:30 PDT (History)
39 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed
fixed
12+
verified


Attachments
WIP Patch (4.44 KB, patch)
2011-11-02 20:29 PDT, Wesley Johnston (:wesj)
no flags Details | Diff | Splinter Review
Patch (5.36 KB, text/plain)
2011-11-03 16:23 PDT, Wesley Johnston (:wesj)
dougt: review-
Details
Screen shots of mobile sites on different Android browsers (610.22 KB, image/png)
2011-11-04 07:04 PDT, Ian Barlow (:ibarlow)
no flags Details
patch v2 (7.01 KB, patch)
2011-11-07 10:26 PST, Wesley Johnston (:wesj)
dougt: review+
Details | Diff | Splinter Review
Patch 1/4 - Add mobile (5.86 KB, patch)
2011-11-08 12:09 PST, Wesley Johnston (:wesj)
dougt: review+
bzbarsky: superreview+
Details | Diff | Splinter Review
Patch 2/4 -> Add android version (1.23 KB, patch)
2011-11-08 12:09 PST, Wesley Johnston (:wesj)
dougt: review+
Details | Diff | Splinter Review
Patch 3/4 -> Add device name (2.56 KB, patch)
2011-11-08 12:13 PST, Wesley Johnston (:wesj)
dougt: review+
Details | Diff | Splinter Review
patch alt (5.26 KB, patch)
2012-01-30 23:40 PST, Mark Finkle (:mfinkle) (use needinfo?)
dougt: review-
gerv: review-
bzbarsky: superreview+
Details | Diff | Splinter Review
patch (gerv) (6.16 KB, patch)
2012-01-31 19:19 PST, Mark Finkle (:mfinkle) (use needinfo?)
dougt: review+
bzbarsky: superreview+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
lukasblakk+bugs: approval‑mozilla‑esr10+
Details | Diff | Splinter Review

Description Al 2011-07-14 12:48:22 PDT
User Agent: Opera/9.80 (Android 3.0.1; Linux; Opera Tablet/ADR-1107051709; U; en) Presto/2.8.149 Version/11.10
Build ID: 20110615151330

Steps to reproduce:

Attempted to redirect the device to a tablet version of a website by matching the common marker "Tablet" in the user agent string.


Actual results:

The device could not be appropriately redirected as the browser UA does not have the keyword "Tablet" in it and due to the absence of tablet dentifiers in the UA, this browser can not systematically be seperately distinguished from a mobile telephone.

The User agent string in this case was:

Mozilla/5.0 (Android; Linux armv7l; rv:2.1.1) Gecko/20110415 Firefox/4.0.2pre Fennec/4.0.1

The device in question was:

Motorola Xoom Tablet


Expected results:

If the "Tablet" identifier would have been present in the user agent string, the device would have properly been identified and redirected.
Comment 1 Mark Finkle (:mfinkle) (use needinfo?) 2011-07-14 20:09:00 PDT
Really? We need "Tablet" in the UA? I don't think we are going to do this. CC'ing for feedback
Comment 2 Kevin Brosnan [:kbrosnan] 2011-07-18 10:18:46 PDT
Very similar to bug 625238. There are better ways to detect the device size and display correct content then adding stuff to our user agent.
Comment 3 Matt Brubeck (:mbrubeck) 2011-08-22 14:05:33 PDT
The stock Android 3.2 browser on Xoom does not include "Tablet" either.  See bug 680886 for further discussion.
Comment 4 Al 2011-08-22 14:18:59 PDT
(In reply to Matt Brubeck (:mbrubeck) from comment #3)
> The stock Android 3.2 browser on Xoom does not include "Tablet" either.  See
> bug 680886 for further discussion.

Yes but the native browser in this case usually has at least "MZ604" or "MZ606" in the UA string which identifies it as the Xoom tablet.
Comment 5 Matt Brubeck (:mbrubeck) 2011-11-02 15:26:02 PDT
Re-opening this.  This is one of the simpler and less-harmful options we have for helping sites send different content for small-screen and large-screen devices.  One downside is that it would require evangelism, since sites are not already looking for "Tablet" in UA headers.
Comment 6 Wesley Johnston (:wesj) 2011-11-02 20:29:20 PDT
Created attachment 571546 [details] [diff] [review]
WIP Patch

This gives us "Tablet" or "Mobile" in the UA String. I only tested on my phone so far, and I think I need to initialize using the pref I added if it exists (or just kill the pref for now probably). Alternatively, we could just use Mobile on phones and nothing on tablets. Cleaner. Shorter. Better?

After talking to mfinkle on irc for a bit, we may try a slightly different option that matches the stock browser better, especially on Native Fennec.
Comment 7 Wesley Johnston (:wesj) 2011-11-03 16:23:59 PDT
Created attachment 571827 [details]
Patch

This includes Android version and the device name in Fennec's UA all the time. It also includes "Mobile" if we think the device is smaller than a tablet. On my phone for instance, I get:

Droid X:
Mozilla/5.0 (Android 2.3.3; Linux armv7l; rv:10.0a1; DROIDX) Gecko/20111103 Firefox/10.0a1 Mobile Fennec/10.0a1

GalaxyTab10.1:
Mozilla/5.0 (Android 3.1; Linux armv7l; rv:10.0a1; GT-P7510) Gecko/20111103 Firefox/10.0a1 Fennec 10.0a1

Brings us closer to being in line with the stock browser's UA:
DroidX:
Mozilla/5.0 (Linux; U; Android 2.3.3; en-us; DROIDX Build 4.5.1_57_DX5-3) AppleWebKit/533.1 (KHTML, like Gecko) Version/4.0 Mobile Safari/533.1

GalaxyTab10.1:
Mozilla/5.0 (Linux; U; Android 3.1; en-us; GT-P7510 Build/HMJ37) AppleWebKit/534.13 (KHTML, like Gecko) Version/4.0 Safari/534.13

Sending review to dougt because most of this is ifdef'd for ANDROID and the pieces that aren't should not affect desktop. If I'm wrong or need someone else (bz?) to look at it I can, but would like to push for this fix in Fennc10-XUL.
Comment 8 Ian Barlow (:ibarlow) 2011-11-04 07:04:44 PDT
Created attachment 571960 [details]
Screen shots of mobile sites on different Android browsers

I'd like to add that this isn't just a problem with tablets -- we need to look at what we are doing on phones, too. 

Right now a lot of sites are serving us their oldest, feature phone optimized versions of their content, while the rest of the browsers on android get the newer smart phone app-like content (see attached). It makes us look pretty bad, and it's one of the top things we hear about in user testing ("why does my twitter/fb/gmail/etc look so crappy on firefox?")

So we need to catch up here as well.
Comment 9 Kevin Brosnan [:kbrosnan] 2011-11-04 07:49:17 PDT
FWIW I tweaked phony to output a UA like comment 7. http://people.mozilla.org/~kbrosnan/tmp/ext/phony.xpi did not help with either of Ian's examples.
Comment 10 Mark Finkle (:mfinkle) (use needinfo?) 2011-11-04 07:51:49 PDT
(In reply to Kevin Brosnan [:kbrosnan] from comment #9)
> FWIW I tweaked phony to output a UA like comment 7.
> http://people.mozilla.org/~kbrosnan/tmp/ext/phony.xpi did not help with
> either of Ian's examples.

Honestly, we don't expect it to. This bug, and the UA change, are about making a UA that is consistent with other browsers on Android. Getting websites to use Mozilla-safe CSS and DOM features is not in the scope of this bug.
Comment 11 Wesley Johnston (:wesj) 2011-11-04 08:51:29 PDT
And providing some indication to sites (BEFORE they send content to us) of whether Fennec is running on a large screen tablet/desktop device, or a small screen like a phone.
Comment 12 Chris Abbott 2011-11-04 09:58:54 PDT
Can I introduce myself into this discussion? I'm Chris Abbott, of DetectRight, and Device Detection systems are my field of expertise. I'm also Al's (see comment 1) device detection provider.

I'm of the opinion that Wesley's patch is a must: it's crucial to have the OS version (look at the difference between Android 0.5 and Android 3.2, for instance), model name (preferably with manufacturer name to provide identifiable search tokens though Android itself already queered that pitch, as has Windows Mobile 7.0/7.5) and possibly a "tablet" or "Mobile" identifier, it's only going to fix things if the detection systems are looking for it. 

Problem 1: getting enough information to the website to allow proper ID for server-side decisions

Problem 2: getting device detection systems to recognise and deal with it.

Problem 3: once detected, getting the website to realise that it's not dealing with the stock browser on the device (first-gen systems such as WURFL or DeviceAtlas aren't good at catering for different browsers while still keeping access to the underlying features of the device). This is a perennial handicap for any browser which isn't the default shipped one, but even afflicts upgraded stock browsers.

As you've realised, withholding information such as OS version, model name, or device type (mobile/tablet) means that a browser is penalised by server-side device detection, which is used on the vast majority of serious mobile sites, if only to provide redirection. 

However, there's another reason for the poor outputs: because most useragents from your browser will be novel to the detection system, in many cases, having the extra information won't help, unless the detection system either contains enough useragent samples to detect it properly to the device (for some systems), or has enough API intelligence to recognise tokens properly. 

I'd add that Android made a very bad decision not to put manufacturer names into the UA as a compulsory thing, since model names are not necessarily unique to one manufacturer: and also, it's much easier for smart code to recognise MOT-DroidX in an otherwise novel useragent than to just recognise "DroidX" and have to interpolate "Android DroidX" to "Motorola DroidX". It's lucky that Android specifies at least that the model name goes before "Build" because otherwise you'd have no way of knowing that a particular token such as "DroidX" was even a model name. So the only way that a useragent would generally gets connected to a device is for someone to log the useragent, put it into a workflow, manually attach the device, and update the DDR.

Unfortunately, Server-side detection is not going to go away, since it's even all about markup or redirection: it's also crucial for analytic purposes, and also for recognising media support: so a content-heavy site visited with Firefox mobile might not know, for instance, that the device supported any video or audio at all.

Most decent mobile experiences have two or three (sometimes four if they have a tablet interface) different websites which are chosen between as a first step before the content is sent. However, that choice of sites is generally done based on resolving the useragent to a model, then resolving the model to a type of device. 

What's happening is one of two things: (1) the site gets detected as a normal web browser (i.e. same behaviour as no detection at all), or (2) the detection system is clever enough to work out that a mobile site is needed, but has no clues whatsoever as to the capability of the phone. To a certain extent, a good detection system would at least know that Mobile Firefox was, for instance, HTML5 capable.

Anyway, point 1: sites generally do not do redirection based on "mobile" or "tablet" appearing in the useragent. This is because a tablet recognition algorithm that just looked for the string "tablet" in the useragent would be useless: simply because of the number of tablets that don't advertise themselves that way, even in the default browser. 

My system DetectRight looks for these tokens as part of a much more sophisticated algorithm to guess whether a useragent is mobile or tablet if other more detailed information is not available. So putting this token into the useragent will suddenly give your users a much better experience on billions of hits a day because my API is already looking for this information.

Most sites are based on either WURFL or DeviceAtlas engines or a framework such as Netbiscuits, Volantis, Wapple, etc. For instance, Facebook uses WURFL. WURFL doesn't honor accept strings: which means that if a device is unrecognised, then many of its advanced features default to "lowest common denominator". That's if it has the device at all, which is by no means certain.

Also, Ad companies know that there's no such thing as a "typical" mobile small screen. Nokia are still releasing 128x160 Series 40 devices, while other companies are releasing HD smartphones: this is why Ads for mobile devices are segregated into different widths. So the picture is really a lot more complicated than just "it's a phone" or "it's a tablet". Hence knowing OS version and man/model gives systems such as mine a chance to adapt accordingly.

If I'm kept informed of changes to Mobile Firefox UAs in this regard, I can at least make sure that my system honors them very quickly, and also help with regards to WURFL support. One problem you have which isn't your fault is that WURFL has recently moved from a GPL to an AGPL licence on its API and imposed conditions on its data file, which means that many companies with WURFLs who don't can't live with AGPL and who don't want to pay for a licence will stick with an aging WURFL which will get progressively worse at detecting everything, not just Firefox Mobile.

If anyone wants more information about particular changes to the useragent, how they would be detected, or what the effect would be in the wider ecosystem, I'd be delighted to chat about it. I've studied all device detection systems, and not just my own, so I know how they're working.

I've written an in-depth white paper about useragent detection in various systems which I can send on request (it's not publicly online yet).

-- Chris
Comment 13 Doug Turner (:dougt) 2011-11-06 21:12:19 PST
Comment on attachment 571827 [details]
Patch


>+    public static String getDeviceType() {

This is a really weird name for a method for what it does.  How about, instead, create a method that returns a boolean and is called 'isTablet'.

You could also put this into the SystemInfo hash as a boolean, right?
Comment 14 Mark Finkle (:mfinkle) (use needinfo?) 2011-11-06 21:47:05 PST
Comment on attachment 571827 [details]
Patch


>+#if defined(ANDROID)
>+    nsCOMPtr<nsIPropertyBag2> infoService = do_GetService("@mozilla.org/system-info;1");
>+    NS_ASSERTION(infoService, "Could not find a system info service");
>+    infoService->GetPropertyAsACString(NS_LITERAL_STRING("deviceType"), mDeviceType);

I would change this variable name to be more explicit about what it can be: "Mobile" or ""
Maybe "mMobileMisc" ?


>diff --git a/xpcom/base/nsSystemInfo.cpp b/xpcom/base/nsSystemInfo.cpp

>             SetPropertyAsAString(NS_LITERAL_STRING("shellVersion"), str);

>+        if (mozilla::AndroidBridge::Bridge()->GetStaticStringField("android/os/Build$VERSION", "RELEASE", str))
>+            SetPropertyAsAString(NS_LITERAL_STRING("osversion"), str);

We already have "version" and "shellVersion". This patch adds "osversion", which arguably should be called "shellVersion", since "shellName" is "Android"

What the hell is "shellVersion" now? From the docs, I assume "Rel (XXX)" where 'XXX' is the SDK version. Seems useless to me. MXR says we don't use "shellVersion" anywhere. Let's change it to be the Android version number. Any objections? If we need the SDK version, add "sdkVersion".

>+        if (mozilla::AndroidBridge::Bridge()->GetDeviceType(str))
>+            SetPropertyAsAString(NS_LITERAL_STRING("deviceType"), str);

I agree with Doug about "deviceType" being a poor name given the values ("" or "Mobile"). Changing this to "isTablet" ("true" or "false" values) is a possible alternative. Either that or change to "displayDPI" and values could be those returned from the Bridge->GetDPI() and we have useful data in all device cases. Thoughts?
Comment 15 Matt Brubeck (:mbrubeck) 2011-11-06 22:07:03 PST
(In reply to Mark Finkle (:mfinkle) from comment #14)
> Either that or change to "displayDPI" and values could
> be those returned from the Bridge->GetDPI() and we have useful data in all
> device cases. Thoughts?

"xlarge" is a size, not a DPI range -- it is based on the physical length and width of the display, not its density.
Comment 16 Doug Turner (:dougt) 2011-11-06 22:32:36 PST
All we need is a predict for the UA String.  isTablet is good enough.
Comment 17 Wesley Johnston (:wesj) 2011-11-07 10:26:28 PST
Created attachment 572518 [details] [diff] [review]
patch v2

Updated patch. Uses an android method "isTablet". Also updates shellVersion to be the Android OS version. I did not change mDeviceType yet. mMobileMisc feels incredibly generic to me, but this is the best I can come up with to describe what this means.

I also included a builid paramter which gives us the Build/4.5.1_57_DX5-3 and Build/HMJ37 sections on the stock browser UA.
Comment 18 Doug Turner (:dougt) 2011-11-07 14:38:22 PST
Comment on attachment 572518 [details] [diff] [review]
patch v2

Review of attachment 572518 [details] [diff] [review]:
-----------------------------------------------------------------

::: embedding/android/GeckoAppShell.java
@@ +1621,5 @@
> +            Configuration config = GeckoApp.mAppContext.getResources().getConfiguration();
> +            // xlarge is defined by android as screens larger than 960dp x 720dp
> +            // and should include most devices ~7in and up.
> +            // http://developer.android.com/guide/practices/screens_support.html
> +            if ((config.screenLayout & Configuration.SCREENLAYOUT_SIZE_MASK) >= Configuration.SCREENLAYOUT_SIZE_XLARGE) {

I don't know if this is the best check to see if a device is a tablet or not, but if it is more or less working, and until there is something better, fine.
Comment 19 Wesley Johnston (:wesj) 2011-11-07 15:06:13 PST
Pushed to inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/31d345404c98
Comment 20 Wesley Johnston (:wesj) 2011-11-07 15:09:41 PST
Altering the bug description to describe what we actually did here.
Comment 21 Wesley Johnston (:wesj) 2011-11-07 15:32:49 PST
And just to be clear, here is the new UA on my two devices:

Tab 10.1: Mozilla/5.0 (Android 3.1; Linux armv7l; rv;10.0a1; GT-P7510 Build/HMJ37) Gecko/20111107 Firefox/10.0a1 Fennec/10.0a1

DroidX: Mozilla/5.0 (Android 2.3.3; Linux armv7l; rv:10.0a1; DROIDX Build/4.5.1_57_DX5-35) Gecko/20111107 Firefox/10.0a1 Mobile Fennec 10.0a1

I can attempt to grab some more if it would be useful?
Comment 22 Chris Abbott 2011-11-07 15:53:22 PST
I hope those UAs in comment 21 are kept! They look good.

> xlarge is defined by android as screens larger than 960dp x 720dp

For what it's worth, there are plenty of Android 7" or above tablets (nearly 50%) with res below that, and a slowly increasing number of < 7" screens with HD resolutions above 960x720 (for instance, Samsung Galaxy S II HD, which has a resolution of 720x1280). 

Chris
Comment 23 Brad Lassey [:blassey] (use needinfo?) 2011-11-07 15:58:38 PST
can we use screen size rather than resolution for this?
Comment 24 Chris Abbott 2011-11-07 16:07:46 PST
If physical screen sizes are available, that's the only way to go. Is it available direct, or would it reverse engineered from resolution and DPI to (eventually) get a diagonal size in inches?

Generally, the 7" cutoff for tablet status seems about right.

Chris
Comment 25 Wesley Johnston (:wesj) 2011-11-07 16:13:12 PST
I'd like to see how this works on a variety of 7" devices if possible first, and maybe get some feedback about whether users want mobile or desktop sites on them. But feel free to file something. We can always modify that algorithm to something better.

Just for those following along, note that isn't a "resolution" in the normal sense (i.e. pixels). Those are in units of dp, which should be independent of dpi (1dp = 1/240in).
Comment 26 Matt Brubeck (:mbrubeck) 2011-11-07 16:21:06 PST
This is already based on (approximate) physical size.  "dp" is a "density-independent pixel"; the ratio of dps to hardware pixels varies depending on the display density, and one dp corresponds to a physical size of around 1/160 of an inch.

(In reply to Chris Abbott from comment #22)
> For what it's worth, there are plenty of Android 7" or above tablets (nearly
> 50%) with res below that, and a slowly increasing number of < 7" screens
> with HD resolutions above 960x720 (for instance, Samsung Galaxy S II HD,
> which has a resolution of 720x1280).

The Galaxy S II HD is 720x1280 hardware pixels, but only 480x853 dp, so the code will do the correct thing here.

There will always be borderline cases in the 5" to 8" range; no matter what we decide, some users will want the opposite, and we can give them that option through prefs or add-ons.
Comment 27 Mark Finkle (:mfinkle) (use needinfo?) 2011-11-07 16:26:06 PST
Piling on: The patch seems to implement what Google recommends: http://android-developers.blogspot.com/2010/12/android-browser-user-agent-issues.html
Comment 28 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2011-11-08 01:10:54 PST
Why is this including the device name in the UA string? Wasn't the conclusion from https://groups.google.com/group/mozilla.dev.platform/browse_thread/thread/43d566ca1333234e clearly against putting the device name in the UA string?

Also, if we are now adding "Mobile", can we get rid of the Fennec token?
Comment 29 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2011-11-08 01:18:12 PST
Could the part that contradicts the decision reached in the newsgroup and in bug 625238 please be reverted? (I.e. the part that exposes the device name in the UA string.)
Comment 30 Jonas Sicking (:sicking) PTO Until July 5th 2011-11-08 01:21:28 PST
There seems to be some pretty big privacy implications here. We should do a privacy review before shipping this.
Comment 31 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2011-11-08 01:33:24 PST
Filed bug 700595 as a follow-up for removing the Fennec token now that we have a more compact and more accurate small form factor indicator.
Comment 32 Ed Morley [:emorley] 2011-11-08 01:40:54 PST
https://hg.mozilla.org/mozilla-central/rev/31d345404c98

Marking tracking Firefox10, since any decisions following on from comment  29 and comment 30 will need to be applied to aurora, once the uplift happens today.

Think this also warrants dev-doc-needed, added for now.
Comment 33 Chris Abbott 2011-11-08 02:07:55 PST
@comment 29: there is lots of new information in this thread about device detection efficacy and useragents, and how that's affected by decisions made on the UA. The decision to expose the exact same model/OS information as in the Android stock browser is the single most important outcome in this entire discussion, IMHO. There is no other single step which will improve outcomes for users of the Mobile Firefox browser. 

Previous discussions about this issue I've seen (including at Opera and Apple) have been based on any number of faulty premises that server-side detection is either unnecessary, markup is king, that it's a bad way to run a website, or that client-side detection/CSS/{insert current hot browser technology of choice} will work fine. This ignores any number of issues from server-side redirection to media and content support, to support of legacy devices and browsers which demand a server-side solution for the long-tail. Server-side detection is a hack, yes: it's messy, difficult and inelegant. But it's unfortunately here to stay. 

The biggest problem that Apple have caused the ecosystem is not allowing server-side detection systems to differentiate between, for instance, an iPhone and an iPhone 4s: to the detriment of the customer in many cases. Ironically, it's now Facebook (which changes the useragent to add their own proprietary information) which gives the best chance of detecting various Apple devices.

Nokia once made a conscious decision to have their Series60 3.0 OSS browsers not differentiate between models. Now they refer to that decision as a "bug" in their current docs.
Comment 34 Dão Gottwald [:dao] 2011-11-08 02:25:52 PST
(In reply to Henri Sivonen (:hsivonen) from comment #28)
> Why is this including the device name in the UA string? Wasn't the
> conclusion from
> https://groups.google.com/group/mozilla.dev.platform/browse_thread/thread/
> 43d566ca1333234e clearly against putting the device name in the UA string?

Yes. The device part should be backed out right now and taken back to the newsgroups.
Comment 35 Dão Gottwald [:dao] 2011-11-08 02:31:22 PST
This also should have gotten Necko peer review and probably super-review too. So I'm going to just back this out.
Comment 36 Ed Morley [:emorley] 2011-11-08 02:47:51 PST
Backed out for comment 28-30, comment 34 and comment 35:
https://hg.mozilla.org/mozilla-central/rev/6e3b8000bdfd
Comment 37 Dão Gottwald [:dao] 2011-11-08 02:51:29 PST
A graphics bug affecting Firefox Mobile belongs in Core:Graphics, a networking bug affecting Firefox Mobile belongs in Core:Networking, etc.. Please stop treating Firefox Mobile as if it was somehow peripheral to Mozilla.
Comment 38 [Baboo] 2011-11-08 06:39:20 PST
(In reply to Chris Abbott from comment #12)
> it's also crucial for analytic purposes

Please discuss this in the newsgroup thread that has created for bug 625238.
Comment 39 Doug Turner (:dougt) 2011-11-08 06:57:45 PST
Dao - regarding the review/sr - clearly this is android only with the exception of adding to the total length of the string.  I am pretty sure that if anyone is an Android platform sr is would be me.  Good idea to get more people reviewing, of course... but lets not pretend that this impacts other platforms.
Comment 40 Doug Turner (:dougt) 2011-11-08 07:01:57 PST
jonas - what specific privacy implications are here?  The default browser already sends these strings.  Happy to fill out paperwork and sit in a privacy review meeting for this change, but "pretty big privacy implications" is extending the truth a bit.
Comment 41 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2011-11-08 07:19:24 PST
(In reply to Doug Turner (:dougt) from comment #39)
> Dao - regarding the review/sr - clearly this is android only with the
> exception of adding to the total length of the string.

This could have consequences beyond Android if we ship Firefox on another OS that initially has no Web developer interest and doesn't get first-class service from device databases. (In fact, Maemo no longer gets first-class service even from Mozilla: The Mobile token wasn't added for Maemo here.)

I think the decision of what the UA string should be like is a Firefox-wide concern and not an Android-specific thing. It's really hard to take cruft out of the requests. See for example bug 572652 getting backed out repeatedly before reaching the release channel. Also, I think wider review is needed when there's a pre-existing newsgroup-discussed WONTFIX on file.

(In reply to Doug Turner (:dougt) from comment #40)
> jonas - what specific privacy implications are here? 

Passive fingerprinting from HTTP.

> The default browser already sends these strings.

If the user is using Firefox, the default browser isn't active to send these strings for the user.
Comment 42 Wesley Johnston (:wesj) 2011-11-08 08:41:39 PST
Just FYI, I did ask for a security review on this yesterday before/as I landed. I was told they looked over the information and had no problems with the change. Given that is most of the reasoning for backing this out, I want to reland it on nightly and aurora.

BUT, given people are worried, I'm going to write a follow up path today that lets you set a pref to override the device information from the UA string. You could fill it in with random fake device information if you want, or set it to "" to ensure no device info is sent to sites.

Sound ok?
Comment 43 Dão Gottwald [:dao] 2011-11-08 08:53:37 PST
(In reply to Doug Turner (:dougt) from comment #39)
> Dao - regarding the review/sr - clearly this is android only with the
> exception of adding to the total length of the string.  I am pretty sure
> that if anyone is an Android platform sr is would be me.

We do not traditionally have peers and super-reviewers per OS. It's all the Mozilla platform. The primary peers here are from the networking stack, even if the device information is extracted somewhere in widget code.

(In reply to Wesley Johnston (:wesj) from comment #42)
> Just FYI, I did ask for a security review on this yesterday before/as I
> landed. I was told they looked over the information and had no problems with
> the change. Given that is most of the reasoning for backing this out, I want
> to reland it on nightly and aurora.

There was a whole bunch of reasons (see above comments and my post in the newsgroups), privacy review was only part of it.

> BUT, given people are worried, I'm going to write a follow up path today
> that lets you set a pref to override the device information from the UA
> string. You could fill it in with random fake device information if you
> want, or set it to "" to ensure no device info is sent to sites.

This sounds like a pretty lousy compromise to me.
Comment 44 Robert Kaiser 2011-11-08 08:56:49 PST
(In reply to Wesley Johnston (:wesj) from comment #42)
> Just FYI, I did ask for a security review on this yesterday before/as I
> landed.

Well, it's not a _security_ but a _privacy_ issue due to the increase of per-HTTP-request data that can be used for fingerprinting.
Comment 45 Mark Finkle (:mfinkle) (use needinfo?) 2011-11-08 09:11:42 PST
(In reply to Wesley Johnston (:wesj) from comment #42)

> BUT, given people are worried, I'm going to write a follow up path today
> that lets you set a pref to override the device information from the UA
> string. You could fill it in with random fake device information if you
> want, or set it to "" to ensure no device info is sent to sites.

This sounds OK to me. We can send device info by default, but allow anyone to remove that bit of data if they choose. Creating an add-on to randomize the "device name" by sending a a name that matches most of your devices physical characteristics, but is not actually your device, sounds easy to do.

(In reply to Dão Gottwald [:dao] from comment #43)

> This sounds like a pretty lousy compromise to me.

But most compromises are.
Comment 46 Gervase Markham [:gerv] 2011-11-08 09:13:58 PST
(In reply to Wesley Johnston (:wesj) from comment #42)
> Just FYI, I did ask for a security review on this yesterday before/as I
> landed. I was told they looked over the information and had no problems with
> the change. Given that is most of the reasoning for backing this out, I want
> to reland it on nightly and aurora.

This is clearly a controversial issue and, given that the technical complexity of the patch is low, and the target branches are mozilla-central and aurora, there is no immediate urgency from a testing or release perspective about having it landed. We need to go back to the discussion group (where discussions happen much better than in Bugzilla) and either come to consensus or, if that is impossible, work out which module owner makes the final decision.

Gerv
Comment 47 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2011-11-08 09:41:52 PST
(In reply to Wesley Johnston (:wesj) from comment #42)
> Given that is most of the reasoning for backing this out, I want
> to reland it on nightly and aurora.

Privacy is not the only reasoning. Please see the newsgroup thread.

> BUT, given people are worried, I'm going to write a follow up path today
> that lets you set a pref to override the device information from the UA
> string. You could fill it in with random fake device information if you
> want, or set it to "" to ensure no device info is sent to sites.

The privacy concern isn't about the site finding out what device you use. It's about creating more uniqueness among Firefox users. If someone sets the device model to "" when it's not "" by default, that makes the users more likely to be identified by fingerprinting. Spoofing another device than the one you have doesn't help with fingerprinting (unless you keep changing the device name), because having other Firefox users distributed across many different device names means they are all more unique than if they all shared one UA string.

Putting fake device model in the string doesn't address the problem than having a longer string on every HTTP request wastes more bytes.

Also, putting the device name in the UA string makes it more feasible to use device characteristic databases, which perpetuates the problem that sites rely on device databases (which disadvantages rare devices when sites do something good for popular devices or disadvantages popular devices when sites do something clueless for popular devices).

Distinguishing only between phone/tablet/desktop (like Opera does) would go a long way giving in to Web authors enough to make it easy to adapt UA-sniffing-oriented adaptations to the three major variants that people are designing for without exposing excessive fingerprinting entropy and without the situation where two equally capable but differently named devices get different content. Users shouldn't have to tweak their UA string on the device name level to optimize it if their device doesn't happen to have the most successful name in its technical capability class.
Comment 48 Gervase Markham [:gerv] 2011-11-08 09:50:36 PST
Guys, this is clearly a discussion with a lot of facets and a bug is not the right place. Can we please head back to mozilla.dev.platform?

Gerv
Comment 49 Wesley Johnston (:wesj) 2011-11-08 12:09:05 PST
Created attachment 572933 [details] [diff] [review]
Patch 1/4 - Add mobile

Splitting this up because I don't want to hold up the entire thing because of one piece. This just adds "Mobile" to our UA on small screen Android devices.
Comment 50 Wesley Johnston (:wesj) 2011-11-08 12:09:48 PST
Created attachment 572934 [details] [diff] [review]
Patch 2/4 -> Add android version

Adds the android version number
Comment 51 Wesley Johnston (:wesj) 2011-11-08 12:13:13 PST
Created attachment 572940 [details] [diff] [review]
Patch 3/4 -> Add device name

Patch 4/4 will add the ability to specify the device name (or any of these) using prefs? I haven't written it yet.

Just to make sure all the i's are dotted, looking for a good sr this time...
Comment 52 Boris Zbarsky [:bz] 2011-11-08 12:21:55 PST
Comment on attachment 572933 [details] [diff] [review]
Patch 1/4 - Add mobile

Probably worth checking the return value of GetPropertyAsBool and defaulting to someting non-random if that fails.

sr=me on just this part with that done, though we should watch out for whatever issues it was Opera ran into with "Mobile" in the UA...  Might be worth contacting them to ask what those were and test against the relevant sites.
Comment 53 Dão Gottwald [:dao] 2011-11-08 12:44:16 PST
> Patch 4/4 will add the ability to specify the device name (or any of these)
> using prefs?

Fwiw, the UA string is intentionally not custumizable via prefs (except for override) since extensions were abusung those prefs we used to allow.
Comment 54 Doug Turner (:dougt) 2011-11-08 14:50:11 PST
Comment on attachment 572934 [details] [diff] [review]
Patch 2/4 -> Add android version

consistent with desktop versioning.
Comment 55 Doug Turner (:dougt) 2011-11-08 14:51:40 PST
Comment on attachment 572940 [details] [diff] [review]
Patch 3/4 -> Add device name

This is what the default browser does, and I am okay with this change.  You must resolve the newsgroup discussion before landing this part.
Comment 56 Dão Gottwald [:dao] 2011-11-08 21:14:30 PST
(In reply to Doug Turner (:dougt) from comment #54)
> Comment on attachment 572934 [details] [diff] [review] [diff] [details] [review]
> Patch 2/4 -> Add android version
> 
> consistent with desktop versioning.

True, but there was some desire to get rid of the version there to combat fingerprinting. We shouldn't add it just because we can. Consistency by itself also isn't compelling. Is there a strong reason to add it?
Comment 57 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2011-11-09 00:23:13 PST
(In reply to Dão Gottwald [:dao] from comment #56)
> (In reply to Doug Turner (:dougt) from comment #54)
> > Comment on attachment 572934 [details] [diff] [review] [diff] [details] [review] [diff] [details] [review]
> > Patch 2/4 -> Add android version
> > 
> > consistent with desktop versioning.
> 
> True, but there was some desire to get rid of the version there to combat
> fingerprinting. We shouldn't add it just because we can. Consistency by
> itself also isn't compelling.

Indeed. I think all additions to the UA string should be backed by really compelling use cases and analysis of potential misuse. It's so hard to take anything out of the UA string that the analysis should happen before stuff is added in the first place.

Also, it's worth noting that desktop Linux builds of the Mozilla suite included the Linux kernel version long ago just because they could. The version number was later removed, because it served no useful purpose but advertised to sites which users were using vulnerable versions. The Linux distro version was deliberately removed starting with Firefox 4 due to fingerprinting concerns.

Furthermore, on platforms where we do expose the OS version, we deliberately don't expose the point release. I didn't build with this patch and test, but by looking at the code, it seems that this would expose the Android point release.

Still further, on platforms where we do expose the OS version, there is the use case of facilitating software downloads. That use case doesn't apply when users get their native apps through the Android Market app instead of downloading apks via Firefox.

That is to say I think we shouldn't expose the Android version.

(In reply to Boris Zbarsky (:bz) from comment #52)
> Comment on attachment 572933 [details] [diff] [review] [diff] [details] [review]
> Patch 1/4 - Add mobile

> sr=me on just this part with that done, though we should watch out for
> whatever issues it was Opera ran into with "Mobile" in the UA...  Might be
> worth contacting them to ask what those were and test against the relevant
> sites.

This patch seems add the lone word "Mobile" outside parentheses. Out of curiosity, why wasn't it put inside the parentheses like other tokens that are not of the form foo/number?

Like this:
Mozilla/5.0 (Android; Linux armv7l; Mobile; rv:10.0a1) Gecko/20111103 Firefox/10.0a1

(In reply to Wesley Johnston (:wesj) from comment #51)
> Created attachment 572940 [details] [diff] [review] [diff] [details] [review]
> Patch 3/4 -> Add device name

I didn't build with this patch, but it seems to me that this would add the device name *after* the rv: token as shown in comment 7.

For legacy sniffing reasons, the rv: token must always be the last token inside the parantheses. (Though I still think the device name shouldn't be added at all.)
Comment 58 Robert Kaiser 2011-11-09 04:35:26 PST
(In reply to Henri Sivonen (:hsivonen) from comment #57)
> This patch seems add the lone word "Mobile" outside parentheses.

Does that comply with the established policy/standard on how UA strings should look? Also, I don't think "Mobile" is a component of the user agent, it's a note about some already stated part of it, so it should be inside the parenthesis.
Comment 59 Mark Finkle (:mfinkle) (use needinfo?) 2011-11-09 05:43:10 PST
(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #58)
> (In reply to Henri Sivonen (:hsivonen) from comment #57)
> > This patch seems add the lone word "Mobile" outside parentheses.
> 
> Does that comply with the established policy/standard on how UA strings
> should look? Also, I don't think "Mobile" is a component of the user agent,
> it's a note about some already stated part of it, so it should be inside the
> parenthesis.

The patch complies with the way Google is suggesting the UA be built:
http://android-developers.blogspot.com/2010/12/android-browser-user-agent-issues.html
Comment 60 Dão Gottwald [:dao] 2011-11-09 05:49:25 PST
(In reply to Mark Finkle (:mfinkle) from comment #59)
> (In reply to Robert Kaiser (:kairo@mozilla.com) from comment #58)
> > (In reply to Henri Sivonen (:hsivonen) from comment #57)
> > > This patch seems add the lone word "Mobile" outside parentheses.
> > 
> > Does that comply with the established policy/standard on how UA strings
> > should look? Also, I don't think "Mobile" is a component of the user agent,
> > it's a note about some already stated part of it, so it should be inside the
> > parenthesis.
> 
> The patch complies with the way Google is suggesting the UA be built:
> http://android-developers.blogspot.com/2010/12/android-browser-user-agent-issues.html

That document is merely descriptive, it doesn't ask us to do the same. "Mobile" is a comment, not a product name, so it should naturally be in the comment section. Is this going to break sites? I think Opera has "Mobi" in the comment section, too.
Comment 61 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2011-11-09 06:09:36 PST
(In reply to Mark Finkle (:mfinkle) from comment #59)
> The patch complies with the way Google is suggesting the UA be built:
> http://android-developers.blogspot.com/2010/12/android-browser-user-agent-
> issues.html

AFAICT, that document is advice to Android vendors about what to send as the UA string of the stock browser. We don't need to comply with it.
Comment 62 Wesley Johnston (:wesj) 2011-11-09 07:39:39 PST
(In reply to Henri Sivonen (:hsivonen) from comment #61)
> (In reply to Mark Finkle (:mfinkle) from comment #59)
> > The patch complies with the way Google is suggesting the UA be built:
> > http://android-developers.blogspot.com/2010/12/android-browser-user-agent-
> > issues.html
> 
> AFAICT, that document is advice to Android vendors about what to send as the
> UA string of the stock browser. We don't need to comply with it.

But it does describe what webdevs should/will/do expect in a UA coming from an Android browser.

(In reply to Henri Sivonen (:hsivonen) from comment #57)
> This patch seems add the lone word "Mobile" outside parentheses. Out of
> curiosity, why wasn't it put inside the parentheses like other tokens that
> are not of the form foo/number?

Again, this is to match the stock browser, and current webdev expectations. I imagine it originated with the phrase "Mobile Safari". The patch I put up makes us say "Mobile Fennec" which is kinda silly I agree. Happy to move it to say "Mobile Firefox" which is more in line with what this is anyway. We could remove "Fennec/xx.xa1", but I imagine that will break the a few sites out there that have been kind enough to add us to their ua sniffing already.

> For legacy sniffing reasons, the rv: token must always be the last token
> inside the parantheses. (Though I still think the device name shouldn't be
> added at all.)

I've never heard of this. Because this is already being used by major mobile browser vendors, even ones that are asking for desktop sites, I'm skeptical it is an issue.

This discussion should move to the newsgroups where it can gain more eyes. Please do so.
Comment 63 Dão Gottwald [:dao] 2011-11-09 08:02:25 PST
(In reply to Wesley Johnston (:wesj) from comment #62)
> > AFAICT, that document is advice to Android vendors about what to send as the
> > UA string of the stock browser. We don't need to comply with it.
> 
> But it does describe what webdevs should/will/do expect in a UA coming from
> an Android browser.

s/an Android browser/the Android browser/. We're not going to match that description, unless your next proposal is to add the Webkit-related cruft and remove the Gecko and Firefox tokens.

> Again, this is to match the stock browser, and current webdev expectations.
> I imagine it originated with the phrase "Mobile Safari".

That's a nice play with words, but it doesn't make much sense. These are two separate tokens.

> The patch I put up
> makes us say "Mobile Fennec" which is kinda silly I agree. Happy to move it
> to say "Mobile Firefox" which is more in line with what this is anyway.

Why would we want this? It won't satisfy sniffers looking for "Mobile Safari".

> > For legacy sniffing reasons, the rv: token must always be the last token
> > inside the parantheses. (Though I still think the device name shouldn't be
> > added at all.)
> 
> I've never heard of this. Because this is already being used by major mobile
> browser vendors, even ones that are asking for desktop sites, I'm skeptical
> it is an issue.

Are you talking about Gecko browsers or those that don't have the rv: token in the first place...?
Comment 64 Robert Kaiser 2011-11-09 15:06:14 PST
OK, looking at http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.43 and the referenced spec for tokens, "Mobile" is a valid product token, though IMHO "Mobile" is a strange product identifier. It fits better as an addition to the comment that specifies more about a product.
Comment 65 Robert Kaiser 2011-11-09 15:13:00 PST
Also note that it should be cleared up how this all works with https://developer.mozilla.org/en/Gecko_user_agent_string_reference - or at least documented there when it's done.
Comment 66 Gervase Markham [:gerv] 2011-11-10 08:06:33 PST
Wesley has started a discussion on this issue in mozilla.dev.planning. Please let's move this over there :-)

Gerv
Comment 67 Dão Gottwald [:dao] 2011-11-10 08:17:49 PST
(In reply to Gervase Markham [:gerv] from comment #66)
> Wesley has started a discussion on this issue in mozilla.dev.planning.
> Please let's move this over there :-)

Yes, except that I'd like to make sure the patch that's supposed to land soon is really what we want.

bz, did you consider the exact position of "Mobile" for your superreview, specifically whether it should be a product token or in the comment section? Do you care at all?
Comment 68 Boris Zbarsky [:bz] 2011-11-10 09:01:48 PST
> bz, did you consider the exact position of "Mobile" for your superreview

Personally, no.

> Do you care at all?

Honestly, no.  I think the attempt to impose rhyme and reason on the meaning of parts of the UA string is more or less doomed to failue; in practice everyone just treats it as magic voodoo...
Comment 69 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2011-11-10 10:13:23 PST
(In reply to Boris Zbarsky (:bz) from comment #68)
> Honestly, no.  I think the attempt to impose rhyme and reason on the meaning
> of parts of the UA string is more or less doomed to failue; in practice
> everyone just treats it as magic voodoo...

If "Mobile" is going to be added right before "Fennec", can we at least drop "Fennec" in the same release cycle before people have a chance to start sniffing for the substring "Mobile Fennec"?
Comment 70 Mark Finkle (:mfinkle) (use needinfo?) 2012-01-30 23:40:21 PST
Created attachment 593001 [details] [diff] [review]
patch alt

This patch is based on the previous patch to add "Mobile" to the Fennec UA. This patch will create the following UA for phones:
"Mozilla/5.0 (Android; Linux armv7l; rv:12.0a1) Gecko/20120131 Firefox/12.0a1 Mobile/12.0a1"

Differences from current UA:
* Changes "Fennec" to "Mobile"

And the following UA on tablets:
"Mozilla/5.0 (Android; Linux armv7l; rv:12.0a1) Gecko/20120131 Firefox/12.0a1"

Differences from current UA:
* Drops the "Mobile" (nee "Fennec") token

Any other changes are common to Mozilla in general, not Fennec. We can adjust other tokens in general, as needed.
Comment 71 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2012-01-31 01:25:41 PST
(In reply to Mark Finkle (:mfinkle) from comment #70)
> "Mozilla/5.0 (Android; Linux armv7l; rv:12.0a1) Gecko/20120131
> Firefox/12.0a1 Mobile/12.0a1"

There was a newsgroup/mailing list discussion about UA strings for Fennec on various device form factors. There was a conclusion reported in that thread: 
Mozilla/5.0 (Android; Mobile; rv:12.0) Gecko/12.0 Firefox/12.0
for phone form factor and 
Mozilla/5.0 (Android; Tablet; rv:12.0) Gecko/12.0 Firefox/12.0
for tablet form factor.

Now this patch implements something different from the conclusion without any explanation why. That'd odd.

Why is this patch not implementing the conclusion of the newsgroup/mailing list discussion?
Comment 72 Gervase Markham [:gerv] 2012-01-31 03:20:05 PST
(Note that, for family reasons, I am currently unavailable to be part of continued discussions on this topic.)

Gerv
Comment 73 Boris Zbarsky [:bz] 2012-01-31 05:40:30 PST
Comment on attachment 593001 [details] [diff] [review]
patch alt

I too would like to understand the relationship of this change and the long discussion we just had about this...
Comment 74 Mark Finkle (:mfinkle) (use needinfo?) 2012-01-31 06:34:02 PST
(In reply to Boris Zbarsky (:bz) from comment #73)
> Comment on attachment 593001 [details] [diff] [review]
> patch alt
> 
> I too would like to understand the relationship of this change and the long
> discussion we just had about this...

See:
http://groups.google.com/group/mozilla.dev.platform/msg/8133920a2ccf4d5b
Comment 75 Mark Finkle (:mfinkle) (use needinfo?) 2012-01-31 06:35:14 PST
One of the main issues is the long term changes were not really mobile-only. If Mozilla wants those changes, we should add them to desktop and mobile at the same time.
Comment 76 Boris Zbarsky [:bz] 2012-01-31 14:22:56 PST
Comment on attachment 593001 [details] [diff] [review]
patch alt

sr=me on the code changes, in that they achieve the stated effect in a reasonable way.

Gerv should be the one reviewing said stated effect.
Comment 77 Gervase Markham [:gerv] 2012-01-31 15:47:44 PST
Comment on attachment 593001 [details] [diff] [review]
patch alt

r- from me, in small part because I'm not happy about the technical effect (e.g. it doesn't match Mozilla's position on limiting OS/platform information in the UA string from a privacy perspective) but in greater part because (without animosity towards the individuals involved, who I'm sure are feeling stuck between a rock and a hard place) I'm unhappy with the process by which the decision to use this string was reached. 

I think there are more stakeholders in this decision than the Fennec team, and more concerns than compatibility, and the Fennec team need to acknowledge that. There was a consensus-building process, which had input from all stakeholders (including the Mobile team, whose strong opinion about inclusion of 'Android' won the day), and a result emerged.

So I think the string which came out of the consensus process, and which was agreed as final at a teleconference meeting which included Stormy, QA and the Mobile team, should be used. To be clear, that is <https://wiki.mozilla.org/Fennec/User_Agent>:

Mozilla/5.0 (Android; Mobile; rv:12.0) Gecko/12.0 Firefox/12.0

(Although if someone has a strong opinion on where "Mobile" should go, I don't - if the Mobile team strongly want it as a bare unversioned product token, that's fine with me - I foresee no effect on anything significant, either way. I am, however, very against a _fourth_ copy of the version number.)

However, even though (as others have said) I've exercised unofficial oversight over the UA strings of Mozilla products for many years, I am also happy to admit that I am not the final authority here. If Brendan tells me that he is happy with the way this has gone down, and that what has happened is how we make decisions, then I'll accept that and withdraw my objection. As the Apostle Paul said: "I appeal to Caesar!"

Now, off to bed and then back to my wife.

Gerv
Comment 78 Doug Turner (:dougt) 2012-01-31 18:47:05 PST
Comment on attachment 593001 [details] [diff] [review]
patch alt

I am not sure why there is any difference between Gerv's proposal and what was implemented.  Please implement Gerv's UA proposal.
Comment 79 Mark Finkle (:mfinkle) (use needinfo?) 2012-01-31 19:19:52 PST
Created attachment 593295 [details] [diff] [review]
patch (gerv)

This create impls Gerv's proposal at: https://wiki.mozilla.org/Fennec/User_Agent

I tested this on a phone and on a tablet. The UA reported on a phone was:
Mozilla/5.0 (Android; Mobile; rv:12.0) Gecko/12.0 Firefox/12.0

The UA reported on a honeycomb tablet was:
Mozilla/5.0 (Android; Tablet; rv:12.0) Gecko/12.0 Firefox/12.0

We have no intention of supporting the "Netbook" variety mentioned in the document at this time.
Comment 80 Boris Zbarsky [:bz] 2012-01-31 19:30:40 PST
Comment on attachment 593295 [details] [diff] [review]
patch (gerv)

sr=me with the same caveats as before.  I'm assuming r=gerv on this one, for obvious reasons.
Comment 81 Brad Lassey [:blassey] (use needinfo?) 2012-01-31 20:06:57 PST
(In reply to Gervase Markham [:gerv] from comment #77)
> So I think the string which came out of the consensus process, and which was
> agreed as final at a teleconference meeting which included Stormy, QA and
> the Mobile team, should be used. To be clear, that is
> <https://wiki.mozilla.org/Fennec/User_Agent>:
> 
> Mozilla/5.0 (Android; Mobile; rv:12.0) Gecko/12.0 Firefox/12.0
Gerv, no decision was reached on that call. That was pretty clear to me anyway, I'm not sure how you came away with that thinking there was agreement.
Comment 82 Doug Turner (:dougt) 2012-01-31 21:00:07 PST
Comment on attachment 593295 [details] [diff] [review]
patch (gerv)

Review of attachment 593295 [details] [diff] [review]:
-----------------------------------------------------------------

This patch correctly implements the Gerv UA proposal.
Comment 83 Doug Turner (:dougt) 2012-01-31 21:03:56 PST
https://hg.mozilla.org/mozilla-central/rev/e18c7bc2c28e
Comment 84 Doug Turner (:dougt) 2012-01-31 21:04:58 PST
Comment on attachment 593295 [details] [diff] [review]
patch (gerv)

requesting m-a m-b approval.
Comment 85 Alex Keybl [:akeybl] 2012-02-02 06:46:10 PST
Comment on attachment 593295 [details] [diff] [review]
patch (gerv)

[Triage Comment]
Native Android user agent work - approved for Aurora 12 and Beta 11.
Comment 86 Matt Brubeck (:mbrubeck) 2012-02-02 15:48:59 PST
This regressed XUL Fennec on ICS tablets.  It should not be pushed to beta without the fix for the regression (bug 723746).
Comment 87 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2012-02-03 06:27:42 PST
Updated https://developer.mozilla.org/en/Gecko_user_agent_string_reference and https://developer.mozilla.org/en/Firefox_13_for_developers

However, if/when this lands of branches, the Firefox N for developers entry needs to be moved accordingly.
Comment 88 Brad Lassey [:blassey] (use needinfo?) 2012-02-06 14:01:00 PST
https://hg.mozilla.org/releases/mozilla-beta/rev/8f30459d86a4
Comment 89 Mark Finkle (:mfinkle) (use needinfo?) 2012-02-06 19:59:59 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/7c3ad9f9c38d
Comment 90 Peter Retzer (:pretzer) 2012-02-09 01:52:47 PST
(In reply to Henri Sivonen (:hsivonen) from comment #87)
> Updated https://developer.mozilla.org/en/Gecko_user_agent_string_reference
> and https://developer.mozilla.org/en/Firefox_13_for_developers
> 
> However, if/when this lands of branches, the Firefox N for developers entry
> needs to be moved accordingly.

So I moved the the documentation to https://developer.mozilla.org/en/Firefox_11_for_developers since it landed on beta.
But since it's not clear atm, if this will really ship as 11 (resceduling for NativeUI), you might want to revert to 'dev-doc-needed' to keep this tracked...
Comment 91 Robert Kaiser 2012-02-09 06:18:00 PST
(In reply to pretzer from comment #90)
> But since it's not clear atm, if this will really ship as 11 (resceduling
> for NativeUI), you might want to revert to 'dev-doc-needed' to keep this
> tracked...

It will ship in 11 in some form, as it's in XUL 11 as well, with one exception: XUL still reports the Fennec/11.0 token appended at the end of the string, so it's e.g.
Mozilla/5.0 (Android; Tablet; rv:11.0) Gecko/11.0 Firefox/11.0 Fennec/11.0
Comment 92 Matt Brubeck (:mbrubeck) 2012-03-08 13:47:26 PST
*** Bug 734068 has been marked as a duplicate of this bug. ***
Comment 93 Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]] 2012-03-26 09:59:29 PDT
removing privacy-review-needed per comment 80 & discussion in dev-planning
Comment 94 Matt Brubeck (:mbrubeck) 2012-03-26 10:16:49 PDT
Comment on attachment 593295 [details] [diff] [review]
patch (gerv)

Requesting approval to land this in mozilla-esr10.  We are currently shipping esr10 to all of our release channel users on Android.  After native Fennec 13 or 14 ships, we plan to continue shipping esr10 to tablet users until some later version with a native tablet UI reaches the release channel.

Landing this on esr10 will ensure that both tablet and phone builds contain this change.  This is important because the original motivation of the change was to allow developers to tell the difference between tablets and phones.  It's also important because we will be evangelizing this change; if we ship it to only a portion of our Android users then it will be difficult for web developers to test and deploy their own changes without breaking some of our users.

Risk analysis:  Very safe.  The patch affects Android only.  (It touches some shared files, but all of the changes are inside of #ifdef ANDROID blocks, or in variables that are used only on Android.)  This change has been on Nightly, Aurora, and Beta for almost two months.  It was included in the Firefox 11 release.

This patch caused one regression in Android-XUL fennec (bug 723746, which has a one-line mobile-only fix that landed on Aurora/Beta at the same time as this patch).  If this patch is approved for esr10, then that regression fix should come with it.

One (theoretically possible) danger of the patch is that some enterprise is using Fennec 10.0esr on Android *and* is using web applications that are broken by the new UA string.  However, I don't think there are any actual enterprise deployments of Android Firefox; our metrics showed no users on the "esr" channel before we deployed ESR builds to the general public through the Android Market two weeks ago.
Comment 95 Lukas Blakk [:lsblakk] use ?needinfo 2012-04-05 16:37:29 PDT
Comment on attachment 593295 [details] [diff] [review]
patch (gerv)

[Triage Comment]
Approving, android-only, low risk
Comment 96 Matt Brubeck (:mbrubeck) 2012-04-06 16:54:33 PDT
https://hg.mozilla.org/releases/mozilla-esr10/rev/34581f8af891
Comment 97 Andreea Pod 2012-04-20 05:24:24 PDT
The User Agent string on Asus Eee Transformers (Android 4.0.3) is: 
Mozilla/5.0 (Android; Tablet; rv:10.0.4esrpre) Gecko/10.0.4es Firefox/10.0.4esrpre Fennec/10.0.4esrpre
Comment 98 Daniel Cater 2012-04-20 08:16:50 PDT
(In reply to Andreea Pod from comment #97)
> The User Agent string on Asus Eee Transformers (Android 4.0.3) is: 
> Mozilla/5.0 (Android; Tablet; rv:10.0.4esrpre) Gecko/10.0.4es
> Firefox/10.0.4esrpre Fennec/10.0.4esrpre

Since when did the user-agent contain an ESR identifier?
Comment 99 dynamis (Tomoya ASAI) 2012-05-17 03:18:47 PDT
(In reply to pretzer from comment #90)
> So I moved the the documentation to
> https://developer.mozilla.org/en/Firefox_11_for_developers since it landed
> on beta.
> But since it's not clear atm, if this will really ship as 11 (resceduling
> for NativeUI), you might want to revert to 'dev-doc-needed' to keep this
> tracked...

You've moved the documentation back to
https://developer.mozilla.org/en/Firefox_13_for_developers
https://developer.mozilla.org/index.php?title=en/Firefox_11_for_developers&action=diff&revision=39&diff=40
https://developer.mozilla.org/index.php?title=en/Firefox_13_for_developers&action=diff&revision=28&diff=29

This bug is still "status-firefox11: fixed". Why you reverted?
Comment 100 Peter Retzer (:pretzer) 2012-05-17 08:22:54 PDT
I reverted because it's not clear to me where it actually belongs.
AFAICS this could be on either of the following MDN pages:
- FF10: because it was back-ported to ESR 10
- FF11: because it's in XUL Fennec 11, which was shipped for tablets only I think(?)
- FF12: because the ESR release this shipped with, shipped in parallel to FF12
- FF14: because it will ship in Native Fennec 14

I'm obviously confused and not the right person to decide... I agree though that FF13 does not really make sense.
Comment 101 Eric Shepherd [:sheppy] 2012-05-17 09:46:58 PDT
OK, I've decided to put this on Firefox 11 for developers, and have updated the marking on the note on the UA reference to correspond to that.
Comment 102 Gervase Markham [:gerv] 2012-06-01 07:36:09 PDT
(In reply to Daniel Cater from comment #98)
> (In reply to Andreea Pod from comment #97)
> > The User Agent string on Asus Eee Transformers (Android 4.0.3) is: 
> > Mozilla/5.0 (Android; Tablet; rv:10.0.4esrpre) Gecko/10.0.4es
> > Firefox/10.0.4esrpre Fennec/10.0.4esrpre
> 
> Since when did the user-agent contain an ESR identifier?

Well, I just checked Firefox 10.0.5 ESR on Linux, and the UA from about:support is:
    Mozilla/5.0 (X11; Linux i686; rv:10.0.5) Gecko/20100101 Firefox/10.0.5

So if this was once wrong, it's now right.

Gerv
Comment 103 henryfhchan 2012-06-14 07:16:28 PDT
Dev doc needed: 
https://developer.mozilla.org/en/Mobile/Firefox_Mobile_for_developers
Comment 104 Matt Brubeck (:mbrubeck) 2012-06-14 08:28:46 PDT
(In reply to henry.fai.hang.chan from comment #103)
> Dev doc needed: 
> https://developer.mozilla.org/en/Mobile/Firefox_Mobile_for_developers

I removed some outdated documentation from that page and added a link to the up-to-date UA reference.

Note You need to log in before you can comment on or make changes to this bug.