Closed Bug 1108555 Opened 5 years ago Closed 5 years ago

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

Categories

(Firefox :: Tabbed Browser, defect)

defect
Not set
Points:
5

Tracking

()

VERIFIED FIXED
Firefox 37
Iteration:
37.2
Tracking Status
e10s + ---

People

(Reporter: c.ascheberg, Assigned: dao)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

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.
tracking-e10s: --- → +
Component: General → Tabbed Browser
*sigh*
Assignee: nobody → dao
Blocks: 1009628
Iteration: --- → 37.2
Points: --- → 5
Flags: qe-verify+
Flags: firefox-backlog+
Keywords: regression
OS: Linux → All
Hardware: x86_64 → All
Depends on: 1050447
Attached patch patch (obsolete) — Splinter Review
Attachment #8534308 - Flags: review?(mconley)
Status: NEW → ASSIGNED
Attachment #8534308 - Attachment is obsolete: true
Attachment #8534308 - Flags: review?(mconley)
Attachment #8537782 - Flags: review?(mconley)
Attachment #8537782 - Flags: review?(jmathies)
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.
Attachment #8537782 - Flags: review?(jmathies) → review+
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.
Attachment #8537782 - Flags: review?(mconley) → review+
(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
https://hg.mozilla.org/mozilla-central/rev/03348b46166b
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
QA Contact: cornel.ionce
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
Status: RESOLVED → VERIFIED
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
(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.
(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
You need to log in before you can comment on or make changes to this bug.