699052 - Android back button should close the selected tab and return to the previous tab when possible

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P2)

Description

[Lawrence Mandel [:lmandel] (use needinfo)]

In the Android native UI, when I select menu->More->Add-ons, about:addons opens in a new tab in my browser. I then click the back button (physical button on my phone) to return to the page that I was viewing (this is the typical flow from preferences in other Android apps) and the browser exits leaving me back at my home screen.

To reproduce:
1. Open any Web page, such as mozilla.org.
2. Select menu button->More->Add-ons.
3. Click the back button.

Expected:
I am returned to the mozilla.org tab.

Actual:
Firefox exits.

Comment 2

[Matt Brubeck (:mbrubeck)]

Attachment: WIP

This is pretty much complete except I haven't really tested it yet.

Comment 3

[Matt Brubeck (:mbrubeck)]

Attachment: WIP 2

Comment 4

[Matt Brubeck (:mbrubeck)]

Attachment: patch

This is not perfect, but I think it's good enough for a first pass.  This implements basically the same behavior as XUL Fennec (which in turn is based partly on both Android's stock browser).  If you press the system Back button and there is no previous history item in the current tab, then:

* If the tab was opened from another activity, Fennec closes the tab and minimizes itself (so you return to the previous activity).

* If the tab was opened from another tab (e.g. by selecting "Open link in new tab" from the context menu), Fennec closes the tab and returns to the "parent tab" if that tab is still open.

* If the tab was opened within Fennec but has no parent tab, or if the parent tab was closed, then Fennec minimizes itself (and returns to the previous activity).

In-content UI like about:firefox and about:addons is always treated as a "child" of the previous tab, so that users can easily dismiss it with the Back button whether or not they noticed that it opened in a new tab.

Comment 5

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

Comment on attachment 582362 [details] [diff] [review]
patch

>Bug 710302 - Don't treat an address with a ":" as a search unless it has a space first [r=mfinkle, a=javascript]

Wrong comment

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

>             case R.id.addons:
>-                GeckoAppShell.sendEventToGecko(new GeckoEvent("about:addons"));
>+                loadUrlInChildTab("about:addons");

The "Child" part of loadUrlInChildTab is not sitting well with me for some unknown reason. Is loadUrlInNewTab ok with you?

>diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js
> nsBrowserAccess.prototype = {
>   openURI: function browser_openURI(aURI, aOpener, aWhere, aContext) {

This method is due for some changes from Kats' patch, but the bitrot won't be too bad for either him or you.

>       gecko: {
>         type: "Tab:Added",
>         tabID: this.id,
>         uri: aURL,
>+        parentId: ("parentId" in aParams) ? aParams.parentId : -1,
>+        external: ("external" in aParams) ? aParams.external : false,
>         selected: ("selected" in aParams) ? aParams.selected : true
>       }
>     };
>+    dump(JSON.stringify(message));

Remove the dump

>diff --git a/mobile/android/components/BrowserCLH.js b/mobile/android/components/BrowserCLH.js

>       if (browserWin) {
>+        dump("openURI with OPEN_EXTERNAL");

Remove the dump

There is some good refactor in this patch too. Let's see what shakes loose so we can get this behavior nailed down.

Comment 6

[Ed Morley [:emorley]]

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

Comment 7

[Brad Lassey [:blassey] (use needinfo?)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e588135f62ab

Comment 8

[Paul Feher]

Verified fixed on:
Nightly Fennec 13.0a1 (2012-03-08)
Device: HTC Desire Z
OS: Android 2.3.3