Last Comment Bug 699052 - Android back button should close the selected tab and return to the previous tab when possible
: Android back button should close the selected tab and return to the previous ...
Status: VERIFIED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: x86 Mac OS X
: P2 normal (vote)
: ---
Assigned To: Matt Brubeck (:mbrubeck)
:
Mentors:
: 709372 (view as bug list)
Depends on: 731610
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-02 08:41 PDT by Lawrence Mandel [:lmandel] (use needinfo)
Modified: 2012-03-12 15:54 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
verified
11+


Attachments
WIP (12.65 KB, patch)
2011-12-13 14:22 PST, Matt Brubeck (:mbrubeck)
no flags Details | Diff | Splinter Review
WIP 2 (14.26 KB, patch)
2011-12-13 16:47 PST, Matt Brubeck (:mbrubeck)
no flags Details | Diff | Splinter Review
patch (18.59 KB, patch)
2011-12-16 12:48 PST, Matt Brubeck (:mbrubeck)
mark.finkle: review+
Details | Diff | Splinter Review

Description Lawrence Mandel [:lmandel] (use needinfo) 2011-11-02 08:41:49 PDT
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 1 Matt Brubeck (:mbrubeck) 2011-12-13 14:01:24 PST
*** Bug 709372 has been marked as a duplicate of this bug. ***
Comment 2 Matt Brubeck (:mbrubeck) 2011-12-13 14:22:08 PST
Created attachment 581433 [details] [diff] [review]
WIP

This is pretty much complete except I haven't really tested it yet.
Comment 3 Matt Brubeck (:mbrubeck) 2011-12-13 16:47:05 PST
Created attachment 581479 [details] [diff] [review]
WIP 2
Comment 4 Matt Brubeck (:mbrubeck) 2011-12-16 12:48:00 PST
Created attachment 582362 [details] [diff] [review]
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?) 2011-12-18 21:44:19 PST
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] 2011-12-20 06:08:22 PST
https://hg.mozilla.org/mozilla-central/rev/e588135f62ab
Comment 7 Brad Lassey [:blassey] (use needinfo?) 2012-01-09 14:42:26 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/e588135f62ab
Comment 8 Paul Feher 2012-03-09 05:59:19 PST
Verified fixed on:
Nightly Fennec 13.0a1 (2012-03-08)
Device: HTC Desire Z
OS: Android 2.3.3

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