768035 - navigator.userAgent property is not changed when requesting desktop site

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Cristian Nicolae (:xti)]

Firefox 16.0a1 (2012-06-25)
Device: Galaxy Nexus
OS: Android 4.0.2

Steps to reproduce:
1. Open Fennec
2. Browse to espn.go.com
3. Tap on Menu > Request Desktop Site

Expected result:
Desktop site version is loaded after step 3.

Actual result:
Mobile version is still displayed after step 3.

Comment 1

[Aaron Train [:aaronmt]]

It's not the same mobile site; it's a less mobile optimized version (i.e, basic HTML).The same thing happens on Stock browser. Requesting the desktop site yields a more basic basic mobile site.

I think this is invalid; there might not be any way around this.

Comment 2

[Brian Nicholson (:bnicholson)]

This happens because ESPN uses navigator.userAgent to perform the mobile redirection in JS. Request Desktop Site changes the UA when requesting HTTP resources, but it doesn't change the navigator.userAgent property.

That property comes from here: http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/nsHttpHandler.cpp#501

To fix this, we need to change this implementation to support tab-based UAs rather than a global preference, or resort to some nasty hack that changes this pref whenever a tab is switched.

The stock browser does work correctly if you manually go to "http://espn.go.com" after switching to desktop mode.

Comment 3

[Wesley Johnston (:wesj)]

I'm going to steal this if you don't mind. We have built in per URI useragent stuff now, so I'm hoping we can use it?

Comment 4

[Brian Nicholson (:bnicholson)]

(In reply to Wesley Johnston (:wesj) from comment #3)
> I'm going to steal this if you don't mind. We have built in per URI
> useragent stuff now, so I'm hoping we can use it?

Is URI fine-grained enough? What is the same page is opened in two different tabs? It seems like we would need a way to set UA per docshell.

Comment 5

[Wesley Johnston (:wesj)]

Attachment: Patch

This should work and brings us back closer in line with what desktop/b2g use. Clobbering...

Comment 6

[Wesley Johnston (:wesj)]

Dang it. Spoke to soon. UserAgentOverrides is called by Navigator.getUserAgent, but only to do the site specific stuff. I'm going to modify it to also do a check based on docshell that we can catch ourselves.

Comment 7

[Wesley Johnston (:wesj)]

Attachment: Patch 1/2

This modifies SitesSpecificUserAgent's getUserAgentForURI to getUserAgentForURIAndDocShell to allow changes to the UA based on either URI or on tab. We made the choice to go with a per-tab request desktop site solution for Fennec at: https://bugzilla.mozilla.org/show_bug.cgi?id=790954#c15. Happy to hear about alternative ways to do this bz.

Comment 8

[Wesley Johnston (:wesj)]

Attachment: Patch 2/2

This implements a custom Fennec version of nsISiteSpecificUseAgent to send user agents based on docshell or uri. It also modifies Fennec to use UserAgentOverrides.jsm like before, so that we're more in line with other Firefox implementations.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 717261 [details] [diff] [review]
Patch 1/2

This seems ok, but why not just use the window instead of the docshell?  Does the callee actually want the docshell?

Comment 10

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 717262 [details] [diff] [review]
Patch 2/2

drive by

>   observe: function ua_observe(aSubject, aTopic, aData) {

>+    let args = JSON.parse(aData);
>+    let tab = BrowserApp.getTabForId(args.tabId);
>+    if (tab != null)
>+      tab.reloadWithMode(args.desktopMode);

Can we keep and if or switch for the "DesktopMode:Change" part so we know what topic the code handles? 
>+        SiteSpecificUserAgent.js \

Forgot to hg add the file?

Comment 11

[Brian Nicholson (:bnicholson)]

Comment on attachment 717262 [details] [diff] [review]
Patch 2/2

Cancelling review for missing SiteSpecificUserAgent.js.

Comment 12

[Wesley Johnston (:wesj)]

Attachment: Patch 1/2

Sorry for the churn bz. I like using windows better as well.

Comment 13

[Wesley Johnston (:wesj)]

Attachment: Patch 2/2

Updated to use window instead. With missing file.

Comment 14

[Wesley Johnston (:wesj)]

Attachment: Patch 2/2

Grr. Removed extra logging.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 718747 [details] [diff] [review]
Patch 1/2

r=me

Comment 16

[Brian Nicholson (:bnicholson)]

Comment on attachment 718760 [details] [diff] [review]
Patch 2/2

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

Nice, looks good!

Comment 17

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0f39615ecb37
https://hg.mozilla.org/integration/mozilla-inbound/rev/5b573c5f2920

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0f39615ecb37
https://hg.mozilla.org/mozilla-central/rev/5b573c5f2920