Incorrect navigator.userAgent requested via Request Desktop Site on Firefox 37

VERIFIED FIXED in Firefox 37

Status

()

Firefox for Android
General
VERIFIED FIXED
3 years ago
2 years ago

People

(Reporter: aaronmt, Assigned: mfinkle)

Tracking

({regression, site-compat})

unspecified
Firefox 40
ARM
Android
regression, site-compat
Points:
---

Firefox Tracking Flags

(firefox37+ verified, firefox38 verified, firefox39 verified, firefox40 verified, relnote-firefox 37+, fennec37+)

Details

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
Currently, Request Desktop Site (pre-enabled) fails to function properly on browser upgrade.

Any further attempts to use Request Desktop Site afterwards seem to fail.

Steps

  Install Firefox 36, enable Request Desktop Site, visit http://www.whatsmyua.com/ 
  Confirm you see a desktop user-agent  

  Upgrade to 37.0.1, see the option enabled, visit http://www.whatsmyua.com/
  Confirm you see a mobile user-agent (!)

  Toggle the setting in the menu, continue to see the mobile user-agent(!)

Seems to have been reported by Japanese users on Play Store comments
Is this reproducible with single-locale en-US builds and/or mainline Nightly?

Do things work correctly if you upgrade with RDS *not* enabled?
Flags: needinfo?(aaron.train)
Keywords: reproducible
status-firefox37: --- → affected
status-firefox38: --- → ?
status-firefox39: --- → ?
status-firefox40: --- → ?
Version: Firefox 39 → unspecified
(Reporter)

Comment 2

3 years ago
Actually, this seems unreliable. Something else must be triggering this. Let's see if others can reproduce.
(Reporter)

Comment 3

3 years ago
I'm nullifying this bug, the cached page tested is throwing things off (whereas something like SUMO works when toggling modes). There must be something else going on.
status-firefox37: affected → ?
Flags: needinfo?(aaron.train)
Keywords: reproducible → steps-wanted
(Reporter)

Comment 4

3 years ago
Problem still stands with reports by Japanese users on Play Store comments. Needs more investigation.
Summary: Request Desktop Site (when pre-enabled) fails to function properly on browser upgrade → Request Desktop Site fails to function?
(Reporter)

Comment 5

3 years ago
I got 100 problems and a user agent is one

http://people.mozilla.org/~atrain/mobile/tests/ua.html

37.0.1 -> flip the RDS setting, same navigator.userAgent reported

40.0 -> flip the RDS setting, the UA flips values
status-firefox37: ? → affected
status-firefox40: ? → unaffected
(Reporter)

Updated

3 years ago
status-firefox38: ? → unaffected
(Reporter)

Updated

3 years ago
Keywords: steps-wanted → regression, regressionwindow-wanted
Summary: Request Desktop Site fails to function? → Incorrect navigator.userAgent requested via Request Desktop Site on Firefox 37
38.0 (beta) -> flip the RDS setting, the UA flips values
Tracking 37 so that we have this on our point release list.
tracking-firefox37: --- → +
I pulled mozilla-release and made a local 37.0.1 build, with official branding. Aaron's test page works fine in that build.

Comment 9

3 years ago
Firefox 38 Beta 2 now affected
pushlog:
https://hg.mozilla.org/releases/mozilla-beta/pushloghtml?fromchange=FENNEC_38_0b1_BUILD1&tochange=FENNEC_38_0b2_BUILD1

Nothing stands out, only Bug 1133862 is about user-agent, but I'm not sure this is what caused the regression
Keywords: regressionwindow-wanted
(Reporter)

Comment 10

3 years ago
Needs debugging at this point, everything that RDS changes should be watched

Comment 11

3 years ago
Tested on Gingerbread device HTC Desire HD (Android 2.3.5).

http://people.mozilla.org/~atrain/mobile/tests/ua.html

UA changes when changing "Request Desktop Site" pref
:aaronmt should i ask the japanese SUMO community for steps to reproduce?
Flags: needinfo?(aaron.train)
Karl, have you heard anything about this? Can you reproduce?
Flags: needinfo?(kdubost)
i pinged marsf and chiko and karl via email, so removing needinfo from aaron
Flags: needinfo?(aaron.train)
(Reporter)

Comment 15

3 years ago
(In reply to Roland Tanglao :rolandtanglao from comment #12)
> :aaronmt should i ask the japanese SUMO community for steps to reproduce?

We have replicated the problem in this bug.
(Reporter)

Comment 16

3 years ago
I can confirm 38.0b1 works and 38.0b2 fails (on my Nexus 6 5.0.1).
(Reporter)

Updated

3 years ago
status-firefox38: unaffected → affected
I can confirm that 37.0.1 on Gingerbread (Nexus S) works fine
OK. As we started to notice, this is a Split APK problem of some kind. Having verified that 37.0.1 api-9 APK works and 37.0.1 api-11 APK fails, I opened up the APKs to compare them.

I had a clue that SiteSpecificUserAgent.js component was failing in api-11, so I compared those files first. Sure enough, there are two different files! how can this be? Easy:

DOM code provides a basic SiteSpecificUserAgent.js that works for Desktop:
http://mxr.mozilla.org/mozilla-release/source/dom/base/SiteSpecificUserAgent.js

Mobile provides a different version, an override, for the "Request Desktop Site" support:
http://mxr.mozilla.org/mozilla-release/source/mobile/android/components/SiteSpecificUserAgent.js

For some reason, api-9 is packaging the Mobile file, but api-11 is packaging the DOM file. I don't know the reason. It might be a nuance of the way those builds are done.

I did spot a problem in the Mobile package.manifest:
http://mxr.mozilla.org/mozilla-release/source/mobile/android/installer/package-manifest.in#328

These entries, the JS and the manifest, are for the DOM code. Mobile should not include those. I can make a patch that leaves out the manifest file (Mobile has it's manifest somewhere else) and move the JS line down into the [mobile] section:
http://mxr.mozilla.org/mozilla-release/source/mobile/android/installer/package-manifest.in#597

I have no idea if this will magically fix the issue, but it will keep the SiteSpecificUserAgent component from showing up twice in the resulting components.manifest file.
Created attachment 8589344 [details] [diff] [review]
package-mobile-sitespecificuseragent v0.1

This patch should have no affect other than removing the DOM SiteSpecificUserAgent manifest entry. The Mobile version SiteSpecificUserAgent.js should still be packaged. I do not know if moving the entry to the [mobile] section will actually fix the bug.

We'll need to test in a Mozilla Build system.

I will push to Try in case it might show there.
Assignee: nobody → mark.finkle
Attachment #8589344 - Flags: review?(rnewman)
(In reply to Mark Finkle (:mfinkle) from comment #18)

> For some reason, api-9 is packaging the Mobile file, but api-11 is packaging
> the DOM file. I don't know the reason. It might be a nuance of the way those
> builds are done.

I suspect it's as simple as a timing/ordering issue. There really isn't much different between the two builds; the API 11 one is how it *used* to be, so this is more that the constrained build got lucky.


> I have no idea if this will magically fix the issue, but it will keep the
> SiteSpecificUserAgent component from showing up twice in the resulting
> components.manifest file.

Can't hurt.
Comment on attachment 8589344 [details] [diff] [review]
package-mobile-sitespecificuseragent v0.1

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

If this is the fix, this is one of those "how did it ever work" situations: if the manifest is using the [browser] section SSUA.js, then how is it including the mobile one in any builds at all?

Looks like this certainly won't make things worse.
Attachment #8589344 - Flags: review?(rnewman) → review+
(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #13)
> Karl, have you heard anything about this? Can you reproduce?

Nope. 
Except a very similar discussion in January 2015: 

Robert Rayborn had said:
> I was wondering if you've heard about any issues with "PC Mode" 
> in the Japanese Firefox for Android.  A large amount of our 
> reviews are about "PC Mode" not working (often when Flash 
> is involved), but I'm not sure if I'm missing details in 
> the translation to English.

Tomoya had replied at this time:
> Kan-colle (http://www.dmm.com/netgame/feature/kancolle.html) 
> is one of most famous Web game in Japan and it only support 
> PC with Flash. Game provider (DMM) don't support but some 
> users use Android Firefox to play Kan-colle with Flash and 
> PC site mode.
>
> AFAIK Fennec have trouble with Flash (ex. Bug 1112086 ).
>
> DMM has changed user-agent detection at the end of last month 
> or beginning of this month to reject Linux UA. Fennec PC site 
> mode use Linux UA, not Win/Mac and some users use phony add-on 
> to avoid the trouble.

Updated

3 years ago
Flags: needinfo?(kdubost)
Most comment seems to be that user cannot play Tohkenranbu (http://www.dmm.com/netgame_s/tohken/) by DMM's flash game.  Although I try to create account for Tohkenranbu, they don't allow it due to server limit.

Comment 24

3 years ago
Created attachment 8589419 [details]
Screenshot_2015-04-08-10-48-36.png

Users claims that users can not play Tokenranbu and Kancolle.
It seems same issue discussed in January as Kerl mentioned.

Process to reproduce:

0. Create account on dmm.com and register yourself to Kancolle
1. Visite http://www.dmm.com/netgame_s/kancolle/ and push the button placed at bottom of the page, of which label is "今すぐ出撃せよ!"
2. Sign-in to Kancolle

Expected result: The game starts.

Actual result: The game display the message "This game is designed for Desktop use, please check your environment." as attached file.
My patch seems to fix the packaging issue. The Try run produces an API-11 APK that has the right SiteSpecificUserAgent.js file.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0be55913b3c2

I compared that to a mozilla-central build, and that API-11 APK has the wrong SiteSpecificUserAgent.js file. The patch will fix that.

Sounds like we might have a second issue around using a nonLinux UA for Request Desktop Site (PC Mode). That can be a new bug.
Comment on attachment 8589344 [details] [diff] [review]
package-mobile-sitespecificuseragent v0.1

Approval Request Comment
[Feature/regressing bug #]: Might be Split APK work. Not conclusive.
[User impact if declined]: navigator.userAgent is broken in "PC Mode", causing a lot of problems for some locales.
[Describe test coverage new/current, TreeHerder]: None yet, but I think we can make one.
[Risks and why]: This patch is very low risk. Minor tweak to packaging that should reduce the probability of packaging the wrong file. 
[String/UUID change made/needed]: none
Attachment #8589344 - Flags: approval-mozilla-release?
Attachment #8589344 - Flags: approval-mozilla-beta?
Attachment #8589344 - Flags: approval-mozilla-aurora?
Comment on attachment 8589344 [details] [diff] [review]
package-mobile-sitespecificuseragent v0.1

Should be in 38 beta 4.
Attachment #8589344 - Flags: approval-mozilla-beta?
Attachment #8589344 - Flags: approval-mozilla-beta+
Attachment #8589344 - Flags: approval-mozilla-aurora?
Attachment #8589344 - Flags: approval-mozilla-aurora+
(Reporter)

Comment 28

3 years ago
Comment on attachment 8589344 [details] [diff] [review]
package-mobile-sitespecificuseragent v0.1

>@@ -616,16 +614,17 @@ bin/libfreebl_32int64_3.so

What is this?
(In reply to Aaron Train [:aaronmt] from comment #28)
> Comment on attachment 8589344 [details] [diff] [review]
> package-mobile-sitespecificuseragent v0.1
> 
> >@@ -616,16 +614,17 @@ bin/libfreebl_32int64_3.so
> 
> What is this?

Just an HG Diff marker
status-firefox39: ? → affected
status-firefox40: unaffected → affected
(Reporter)

Comment 30

3 years ago
40 was unaffected. We should figure out why.
status-firefox40: affected → unaffected
(Reporter)

Comment 31

3 years ago
39 was unaffected. We should figure out why.
status-firefox39: affected → unaffected
(In reply to Aaron Train [:aaronmt] from comment #30)
> 40 was unaffected. We should figure out why.

That might be true, but this change needs to land on 40
status-firefox39: unaffected → affected
status-firefox40: unaffected → affected
https://hg.mozilla.org/mozilla-central/rev/256b307c35ce
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox40: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 40
(Assignee)

Updated

3 years ago
tracking-fennec: ? → 37+
(In reply to Aaron Train [:aaronmt] from comment #30)
> 40 was unaffected. We should figure out why.

I don't think there's a pattern. This is a build race.
I tested Fx38b3 and it seems to be working fine.

https://ftp.mozilla.org/pub/mozilla.org/mobile/releases/38.0b3/android-api-11

Let's wait and see what QA says, but we might be good to try a Fx37 point release.

Comment 40

3 years ago
I've tested this issue and the behavior is inconsistent.
On Firefox 38 Beta 3 on Nexus 5(Android 5.1):
 * http://people.mozilla.org/~atrain/mobile/tests/ua.html -> UA changes when changing "Request Desktop Site" pref
 * https://www.mozilla.org/en-US -> no changes in the page display

On latest Nightly after fix, on Nexus 5(Android 5.1)
 * http://people.mozilla.org/~atrain/mobile/tests/ua.html -> UA does not change when changing "Request Desktop Site" pref
 * https://www.mozilla.org/en-US -> changed to desktop site

Still investigating
(In reply to Mihai Pop from comment #40)
> I've tested this issue and the behavior is inconsistent.
> On Firefox 38 Beta 3 on Nexus 5(Android 5.1):
>  * http://people.mozilla.org/~atrain/mobile/tests/ua.html -> UA changes when
> changing "Request Desktop Site" pref
>  * https://www.mozilla.org/en-US -> no changes in the page display
> 
> On latest Nightly after fix, on Nexus 5(Android 5.1)
>  * http://people.mozilla.org/~atrain/mobile/tests/ua.html -> UA does not
> change when changing "Request Desktop Site" pref
>  * https://www.mozilla.org/en-US -> changed to desktop site
> 
> Still investigating

Instead of https://www.mozilla.org/en-US try using http://whatsmyua.com

We should be testing the raw APIs. whatsmyua.com shows the HTTP header and Aaron's page tests the navigator.userAgent API.

Why is mozilla.org not changing? I don't know right now, but it could be non-UA related. It might be a responsive design based on the screen size.
My Galaxy Nexus and Nexus 5 seem to be working correctly using Fx38b3 and latest Nightly.

Comment 43

3 years ago
Marking this as verified since the UA changes(In reply to Mark Finkle (:mfinkle) from comment #41)
> (In reply to Mihai Pop from comment #40)
> > I've tested this issue and the behavior is inconsistent.
> > On Firefox 38 Beta 3 on Nexus 5(Android 5.1):
> >  * http://people.mozilla.org/~atrain/mobile/tests/ua.html -> UA changes when
> > changing "Request Desktop Site" pref
> >  * https://www.mozilla.org/en-US -> no changes in the page display
> > 
> > On latest Nightly after fix, on Nexus 5(Android 5.1)
> >  * http://people.mozilla.org/~atrain/mobile/tests/ua.html -> UA does not
> > change when changing "Request Desktop Site" pref
> >  * https://www.mozilla.org/en-US -> changed to desktop site
> > 
> > Still investigating
> 
> Instead of https://www.mozilla.org/en-US try using http://whatsmyua.com
> 
> We should be testing the raw APIs. whatsmyua.com shows the HTTP header and
> Aaron's page tests the navigator.userAgent API.
> 
> Why is mozilla.org not changing? I don't know right now, but it could be
> non-UA related. It might be a responsive design based on the screen size.

It seems that the UA changes when changing "Request Desktop Site" pref on latest builds.
Marking this as verified.
status-firefox38: fixed → verified
status-firefox39: fixed → verified
status-firefox40: fixed → verified
Comment on attachment 8589344 [details] [diff] [review]
package-mobile-sitespecificuseragent v0.1

We'll take this in Fennec 37.0.2. Release+
Attachment #8589344 - Flags: approval-mozilla-release? → approval-mozilla-release+

Comment 46

3 years ago
Verified as fixed on Firefox 37.0.2
Status: RESOLVED → VERIFIED
status-firefox37: fixed → verified
relnoted as "Request Desktop Site feature does not work as expected".
relnote-firefox: --- → 37+

Updated

3 years ago
Keywords: site-compat
The "sections" in package-manifest.in are almost entirely useless and are remnants of ancient times (except when there is a destdir, which is not the case here.

The actual problem is that both dom/base/moz.build and mobile/android/components/moz.build give "SiteSpecificUserAgent.js" in EXTRA_COMPONENTS, which means building in either directory copies the file in dist/bin/components. Whichever happens last wins.

Moreover, the manifest is necessary to register the component, and the mobile version doesn't have a manifest. What happened before is that the dom/base manifest was installed and that kind of worked, even though the class id doesn't match what's in the .js file (the component manager probably doesn't care).

Anyways, what this means is that what the patch did, simply is:
a) not fix the build problem: the SiteSpecificUserAgent.js file likely still doesn't match.
but
b) it removed SiteSpecificUserAgent.manifest, so the component is not registered anymore

IOW, it broke whatever expects the component to work.
(In reply to Mike Hommey [:glandium] from comment #48)
> The "sections" in package-manifest.in are almost entirely useless and are
> remnants of ancient times (except when there is a destdir, which is not the
> case here.
> 
> The actual problem is that both dom/base/moz.build and
> mobile/android/components/moz.build give "SiteSpecificUserAgent.js" in
> EXTRA_COMPONENTS, which means building in either directory copies the file
> in dist/bin/components. Whichever happens last wins.

We might need to block the DOM version when building Fennec. Or use a different name. Or something less hacky.

> 
> Moreover, the manifest is necessary to register the component, and the
> mobile version doesn't have a manifest. What happened before is that the
> dom/base manifest was installed and that kind of worked, even though the
> class id doesn't match what's in the .js file (the component manager
> probably doesn't care).
> 
> Anyways, what this means is that what the patch did, simply is:
> a) not fix the build problem: the SiteSpecificUserAgent.js file likely still
> doesn't match.
> but
> b) it removed SiteSpecificUserAgent.manifest, so the component is not
> registered anymore

We do have a manifest for it:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/components/MobileComponents.manifest#88
(In reply to Mark Finkle (:mfinkle) from comment #49)
> We do have a manifest for it:
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/components/
> MobileComponents.manifest#88

So the patch didn't change anything then. If the new builds are "fixed", it's in the same way that some versions could be working before, i.e. by chance.

If you rename the mobile file and don't install the dom manifest, the renamed file will be what matters (and the dom file will be there, but not used). You can also, obviously, not install the dom file at all for mobile builds.
(Assignee)

Updated

3 years ago
Depends on: 1154960

Comment 51

3 years ago
It seems that this issue still affects Firefox 38 Beta 4 and Beta 6; is fixed on Firefox 38 Beta 3.
Tested on:
http://www.whatsmyua.com/
http://people.mozilla.org/~atrain/mobile/tests/ua.html
(Reporter)

Comment 52

3 years ago
(In reply to Mihai Pop from comment #51)
> It seems that this issue still affects Firefox 38 Beta 4 and Beta 6; is
> fixed on Firefox 38 Beta 3.
> Tested on:
> http://www.whatsmyua.com/
> http://people.mozilla.org/~atrain/mobile/tests/ua.html

We need a proper fix.
You need to log in before you can comment on or make changes to this bug.