766406 - Implement "Request Desktop Site"

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Brian Nicholson (:bnicholson)]

We previously had the ability to switch to a desktop site (bug 697857), but removed it mainly because of redirection issues (bug 706264).

Here's the problem:
1) A user goes to a site, say http://www.yahoo.com
2) The user is automatically redirected to a mobile version of the site, http://m.yahoo.com
3) The user clicks Request Desktop Site
4) The UA is changed and the site reloads, but the user is still at http://m.yahoo.com

Switching to desktop mode was technically working, but it left users looking at a mobile page with a desktop UA. This made it appear as if UA switching wasn't working at all.

To worsen this problem, we wanted site-specific UA settings. Sites were partitioned by host (m.yahoo.com or www.yahoo.com). So after landing on http://m.yahoo.com, if the user tries going to http://www.yahoo.com, the UA mode is reset to mobile since the host changed. Chrome implements this feature, but the UA setting is tab-specific, so the page does at least update if the user manually re-navigates to the page.

Unfortunately, there aren't any great solutions for translating a mobile URL to its desktop equivalent. If we kept track of past redirects (a solution proposed in https://bugzilla.mozilla.org/show_bug.cgi?id=706264#c1), the pre-redirected URL becomes stale as the user clicks links to navigate the site.

Rather than reloading the current URL with the desktop UA, we could just go directly to that site's root URL, removing the path. This will take the user off the page they were looking at, but if they want to see the page in desktop mode, they generally have to do this anyway, as mentioned above. It's not perfect, but it does consistently load desktop versions of sites.

Comment 1

[Brian Nicholson (:bnicholson)]

Attachment: Implement "Request Desktop Site"

This is a heavily modified version of the patch from bug 697857 with the following changes:
- the "root URL" is loaded when Request Desktop Site is clicked, as mentioned in comment 0
- the desktop UA is automatically generated
- the UA is saved in localStorage for each site

I've tried it on several sites (google.com, yahoo.com, mozilla.org), and it seems to work pretty well.

Comment 2

[Brian Nicholson (:bnicholson)]

Here's a build containing this change:
http://dl.dropbox.com/u/35559547/fennec-request-desktop-site.apk

Comment 3

[Wesley Johnston (:wesj)]

Why is so much of this in Java? Can't we use our own menu apis?

Comment 4

[Brian Nicholson (:bnicholson)]

(In reply to Wesley Johnston (:wesj) from comment #3)
> Why is so much of this in Java? Can't we use our own menu apis?

We probably could, though I think we'd have to add some things, like the ability to change a menu item's label and disable/enable an item. I'm also not sure how we could refresh the menu state in onPrepareOptionsMenu() without maintaining some agentMode state on the Java side.

Comment 5

[Aaron Train [:aaronmt]]

re comment #0, do we know what stock browser does?

Comment 6

[Brian Nicholson (:bnicholson)]

(In reply to Aaron Train [:aaronmt] from comment #5)
> re comment #0, do we know what stock browser does?

It looks like the stock browser and Chrome keep track of the original pre-redirected URLs. When switching the UA, it loads these URLs. We could do this too since it does work pretty well in a number of cases (clicking Google results, news items on yahoo.com, etc). We'll have to make the assumption that the redirection is from a desktop URL -> mobile URL, which isn't always true, but should be in most cases. Combined with the implementation from comment 0, I think this can work pretty well.

Comment 7

[Brian Nicholson (:bnicholson)]

Attachment: Implement "Request Desktop Site", v2

Implementing this as a persistent site-specific setting introduces too many problems, so this just makes it a non-persistent, tab-specific setting.

This keeps track of the original URL, as described in comment 6, so that switching modes keeps the user on the same page when possible.

Comment 8

[Brian Nicholson (:bnicholson)]

Attachment: Implement "Request Desktop Site", v2.1

forgot to refresh some changes

Comment 9

[Wesley Johnston (:wesj)]

Hey brian, you mind filing bugs for adding those menu features you mentioned?

Comment 10

[Brian Nicholson (:bnicholson)]

(In reply to Wesley Johnston (:wesj) from comment #9)
> Hey brian, you mind filing bugs for adding those menu features you mentioned?

Filed bug 767213.

Comment 11

[Brian Nicholson (:bnicholson)]

Attachment: Add preference to request desktop site by default

Since we don't save desktop settings for each site, this setting could be useful to some users (particularly tablet users).

Comment 12

[Brian Nicholson (:bnicholson)]

Attachment: Add preference to request desktop site by default

wrong patch

Comment 13

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

Comment on attachment 635528 [details] [diff] [review]
Implement "Request Desktop Site", v2.1

>diff --git a/mobile/android/base/BrowserApp.java b/mobile/android/base/BrowserApp.java

>+        desktopMode.setEnabled(true);
>+        desktopMode.setCheckable(true);

Why not set these two in the XML?

>diff --git a/mobile/android/base/GeckoApp.java b/mobile/android/base/GeckoApp.java

>+            case R.id.desktop_mode:
>+                Tab selectedTab = Tabs.getInstance().getSelectedTab();
>+                if (selectedTab == null)
>+                    return true;
>+                JSONObject args = new JSONObject();
>+                try {
>+                    args.put("desktopMode", !item.isChecked());
>+                    args.put("tabId", selectedTab.getId());
>+                } catch (JSONException e) {
>+                    Log.e(LOGTAG, "error building json arguments");
>+                }
>+                GeckoAppShell.sendEventToGecko(GeckoEvent.createBroadcastEvent("DesktopMode:Change", args.toString()));
>+                return true;

It kinda dawns on me that this code should probably be in BrowserApp somehow. Later patch...

>diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js

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

>+        if (channel.loadFlags & Ci.nsIChannel.LOAD_DOCUMENT_URI) {
>+          // Remember original URL for UA changes on redirected pages
>+          tab.originalURL = channel.originalURI.spec;

Normally I favor holding the URI

>+          // Send desktop UA if "Request Desktop Site" is enabled
>+          if (tab.desktopModeEnabled)

Can we rename to "desktopMode"? The "Enabled" suffix makes it too wordy for me and "desktopMode" works as a boolean.

>+  reloadDesktopMode: function (aDesktopMode) {

OCD nit: rename to reloadWithMode ?

>+    let uri = this.browser.currentURI;

it's nice to know uri is currentURI, so let's rename the local var

>+    let flags = Ci.nsIWebNavigation.LOAD_FLAGS_BYPASS_CACHE;
>+    if (this.originalURL && this.originalURL != url) {

If we change originalURL to originalURI the comparison would be | this.originalURI.equals(currentURI) |

>+      // Many sites use mobile-specific URLs, such as:
>+      //   http://m.yahoo.com
>+      //   http://www.google.com/m
>+      // If the user clicks "Request Desktop Site" while on a mobile site, it
>+      // will appear to do nothing since the mobile URL is still being
>+      // requested. To address this, we do the following:
>+      //   1) Remove the path from the URL (http://www.google.com/m?q=query -> http://www.google.com)
>+      //   2) If a host subdomain is "m", remove it (http://en.m.wikipedia.org -> http://en.wikipedia.org)
>+      // This means the user is sent to site's home page, but this is better
>+      // than the setting having no effect at all.
>+      if (aDesktopMode) {
>+        url = Services.io.newURI(this.browser.currentURI.spec, null, null)
>+                         .prePath.replace(/([\/\.])m\./g, "$1");

one line is OK and this is borderline hacky, but I am OK to try it out.

r- just to see a new patch

Comment 14

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

Comment on attachment 635542 [details] [diff] [review]
Add preference to request desktop site by default

I don't think we want this. In the long run, the tablet UA is different than the phone, and we want websites to use the UA to direct.

You can get UX to weigh in too, but I don't think we want to do this part.

Comment 15

[Brian Nicholson (:bnicholson)]

Attachment: Implement "Request Desktop Site", v3

with suggested changes

Comment 16

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

Comment on attachment 635641 [details] [diff] [review]
Implement "Request Desktop Site", v3

\o/

Comment 17

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

I just thought of something: Do we need this for tablets? They should (hopefully) be getting desktop sites. Well, we can wait and see.

Comment 18

[Brian Nicholson (:bnicholson)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/aaff24d3411a

Comment 19

[Ryan VanderMeulen [:RyanVM]]

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

Comment 20

[Aaron Train [:aaronmt]]

Verified Fixed, Nightly (06.24)

Need an owner for this new feature on MozTrap.

Comment 21

[Brian Nicholson (:bnicholson)]

Attachment: patch for aurora

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: users cannot switch between mobile/desktop UA
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): medium-low risk; fairly isolated feature, but hasn't been thoroughly tested
String or UUID changes made by this patch: menu label for "Request Desktop Site"

Comment 22

[Brian Nicholson (:bnicholson)]

http://hg.mozilla.org/releases/mozilla-aurora/rev/65e39bd76d91

Comment 23

[Aaron Train [:aaronmt]]

Verified Fixed on Aurora (07/10)