1108555 - [e10s] closing last tab does not focus urlbar (browser.tabs.closeWindowWithLastTab = false)

Status: VERIFIED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Christian Ascheberg]

STR:
- enable e10s
- set browser.tabs.closeWindowWithLastTab = false
- close all tabs in a window

Result:
- after the last tab was closed, a new empty tab will be opened, but the urlbar is not focused

Expected:
- the urlbar should be focused

This does not happen if e10s is disabled.

Comment 1

[Dão Gottwald [:dao]]

*sigh*

Comment 2

[Dão Gottwald [:dao]]

Attachment: patch

Comment 3

[Dão Gottwald [:dao]]

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=0986b6cb6a54

Comment 4

[Dão Gottwald [:dao]]

Attachment: patch, updated to fx-team tip

Comment 5

[Jim Mathies [:jimm]]

Comment on attachment 8537782 [details] [diff] [review]
patch, updated to fx-team tip

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

Solves the focus problem for me.

I'm not sure though what our address bar focus policy is supposed to be. I see:

1) tabs with addresses loaded do not get focus in the address bar
2) tabs that point to internal pages people navigate away from like about:blank or about:newtab get focus in the address bar
3) tabs that point to internal pages with text inputs like about:home get focus in the text input

flipping between older tabs seems to remember and set the last state of the particular tab.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8537782 [details] [diff] [review]
patch, updated to fx-team tip

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

Yes, I like this solution better of the _skipContentFocus expando. Good thinking. :)

::: browser/base/content/tabbrowser.xml
@@ +1190,5 @@
>                  // We need to explicitly focus the new tab, because
>                  // tabbox.xml does this only in some cases.
>                  this.mCurrentTab.focus();
> +              } else if (gMultiProcessBrowser) {
> +                document.activeElement.blur();

Let's add a comment here about why we're blurring this - so that we can easily detect if the active element has changed by the time _adjustFocusAfterTabSwitch is called.

Comment 7

[Dão Gottwald [:dao]]

(In reply to Jim Mathies [:jimm] from comment #5)
> Comment on attachment 8537782 [details] [diff] [review]
> patch, updated to fx-team tip
> 
> Review of attachment 8537782 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Solves the focus problem for me.
> 
> I'm not sure though what our address bar focus policy is supposed to be. I
> see:
> 
> 1) tabs with addresses loaded do not get focus in the address bar
> 2) tabs that point to internal pages people navigate away from like
> about:blank or about:newtab get focus in the address bar
> 3) tabs that point to internal pages with text inputs like about:home get
> focus in the text input
> 
> flipping between older tabs seems to remember and set the last state of the
> particular tab.

All of the above is roughly correct, although it's probably not quite a comprehensive verbalization of our policy as implemented :)

(In reply to Mike Conley (:mconley) - Needinfo me! from comment #6)
> > +              } else if (gMultiProcessBrowser) {
> > +                document.activeElement.blur();
> 
> Let's add a comment here about why we're blurring this - so that we can
> easily detect if the active element has changed by the time
> _adjustFocusAfterTabSwitch is called.

Yes, done.

https://hg.mozilla.org/integration/fx-team/rev/03348b46166b

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/03348b46166b

Comment 9

[Cornel Ionce [:noni] [Hubs QA]]

Reproduced this issue with 2014-12-10 Nightly.

Verified fixed with e10s enabled using latest Nightly, build ID: 20141221030204.
Tested on Windows 7 64-bit, Ubuntu 12.04 32-bit and Mac OS X 10.9.5

User Agents:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:37.0) Gecko/20100101 Firefox/37.0
Mozilla/5.0 (X11; Linux i686; rv:37.0) Gecko/20100101 Firefox/37.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:37.0) Gecko/20100101 Firefox/37.0

Comment 10

[Nicholas Nethercote [inactive]]

This partly broke the Tree Style Tabs extension.

Prior to this change, when you middle-click on a link, the new tab would be opened as a child of the current tab. After this change, the new tab gets opened as a new top-level tab at the bottom of the tab list. This makes Tree Style Tab much less pleasant to use.

I'm using the version from here: http://piro.sakura.ne.jp/xul/xpi/nightly/treestyletab.xpi

Comment 11

[Dão Gottwald [:dao]]

(In reply to Nicholas Nethercote [:njn] from comment #10)
> This partly broke the Tree Style Tabs extension.
> 
> Prior to this change, when you middle-click on a link, the new tab would be
> opened as a child of the current tab. After this change, the new tab gets
> opened as a new top-level tab at the bottom of the tab list. This makes Tree
> Style Tab much less pleasant to use.
> 
> I'm using the version from here:
> http://piro.sakura.ne.jp/xul/xpi/nightly/treestyletab.xpi

Please report this to the author of Tree Style Tabs.

Comment 12

[Nicholas Nethercote [inactive]]

(In reply to Dão Gottwald [:dao] from comment #11)
> 
> Please report this to the author of Tree Style Tabs.

I filed https://github.com/piroor/treestyletab/issues/822