646297 - The next child tab is not focused when closing tabs, the parent tab is focused instead.

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[u88484]

The next child tab should be focused when closing tabs opened from a parent tab.  On Fennec the parent tab is always focused where as Firefox the next child tab is focused until all child tabs have been closed and then the parent tab is focused.

Example:

1) Open cnn.com
2) Open two links in a new tab
3) Go to the third tab
4) Close the third tab and the first tab (main cnn.com page) is focused

The second tab that was spawned from the main cnn.com page should be focused, not the first tab

This makes opening a few tabs from one page a pain to read as when closing each tab the first tab is focused and you have to go to the tabstrip each time to get to the next story.

Firefox controls this with the preference 'browser.tabs.selectOwnerOnClose' which is defaulted to true. The preference name is kind of confusing though as it sounds like the owner is always selected when in face the sibling tabs are first selected until there are no more.

It appears bug 515409 changed this on Fennec or might have implemented this behavior wrong.

Comment 1

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Note: any focused child tab that is closed will return focus on the parent tab.
ie spawn 3 child tabs off of a link from a parent tab, go to the last child tab and close it.  focus will be on the parent tab.

Comment 2

[Matt Brubeck (:mbrubeck)]

Attachment: patch

On closing a tab, switch to other "sibling" tabs if there are any, before going back to the opener.  This is closer to desktop Firefox behavior (though it still doesn't exactly emulate the desktop logic, which is rather more complex).

This is especially useful for the common case where you go to a page with lots of links (e.g. a search result page) and open several of them in tabs to read in sequence.

This patch preserves the behavior of the Android back/escape key; it will still return directly to the opener tab if there is one.

Comment 3

[Matt Brubeck (:mbrubeck)]

Attachment: patch v2

Now with a browser-chrome test!

Comment 4

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

Comment on attachment 528471 [details] [diff] [review]
patch v2


>     // Only if there are no dialogs, popups, or panels open
>     let tab = Browser.selectedTab;
>     let browser = tab.browser;
> 
>     if (browser.canGoBack) {
>       browser.goBack();
>     } else if (tab.owner) {
>+      Browser.selectedTab = tab.owner;
>       this.closeTab(tab);
>     }

Why is this needed?

Comment 5

[Matt Brubeck (:mbrubeck)]

(In reply to comment #4)
> >     } else if (tab.owner) {
> >+      Browser.selectedTab = tab.owner;
> >       this.closeTab(tab);
> >     }
> 
> Why is this needed?

This is in handleEscape, which is called when Android's Back button is pressed.  I wanted to keep the current behavior of the Back button unchanged:  When you press Back in a tab with an owner, it closes the tab and goes back to the owner.

Comment 6

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

Comment on attachment 528471 [details] [diff] [review]
patch v2

>     if (browser.canGoBack) {
>       browser.goBack();
>     } else if (tab.owner) {
>+      Browser.selectedTab = tab.owner;
>       this.closeTab(tab);
>     }

Add a comment in the block as to why we are selecting the tab's owner

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

>     let nextTab = this._selectedTab;
>     if (nextTab == aTab) {
>-      nextTab = aTab.owner || this.getTabAtIndex(tabIndex + 1) || this.getTabAtIndex(tabIndex - 1);
>+      nextTab = this.getTabAtIndex(tabIndex + 1) || this.getTabAtIndex(tabIndex - 1);
>+      if (aTab.owner && nextTab.owner != aTab.owner)
>+        nextTab = aTab.owner;

Add a comment in the block so we know the code is looking (not very hard) for a sibling of the same owner, then defaulting to the owner.

Test looks good.

Comment 7

[Matt Brubeck (:mbrubeck)]

http://hg.mozilla.org/mozilla-central/rev/1ed62cc2e217

Comment 8

[Anna (Waverley)]

VERIFIED FIXED on:
Build Id: Mozilla /5.0 (Android;Linux armv7l;rv:6.0a1) Gecko/20110427 Firefox/6.0a1 Fennec /6.0a1 

Device: HTC Desire Z (Android 2.2)