Closed Bug 1069089 Opened 10 years ago Closed 10 years ago

[e10s] Location bar gets focus when switching back to the previous tab after opening a new one

Categories

(Firefox :: General, defect)

defect
Not set
normal
Points:
8

Tracking

()

VERIFIED FIXED
Firefox 35
Iteration:
35.3
Tracking Status
e10s m3+ ---

People

(Reporter: billm, Assigned: dao)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

I've been trying to pin down an STR for a while, but I just can't figure out one. The basic problem is that I'll switch tabs and, randomly, the address bar will be in focus. I use the arrow keys to scroll fairly often, so it's pretty annoying for me.
Aha! After looking at the patch in bug 1050447 (which seemed like a likely candidate to be the regressor), I figured out what's going on.

STR:
1. Select some tab and make sure content is in focus.
2. Open a new tab with Ctrl-N or whatever.
3. Go back to the original tab. The URL bar will now be in focus.

Mike, do you think you have time to take this? It's driving me crazy.
Blocks: 1050447
Flags: needinfo?(mconley)
OS: Linux → All
Attached patch url-bar-issue (obsolete) — Splinter Review
Actually, I couldn't help but fix this now that I know what it is. I actually took an approach you discarded:
https://bugzilla.mozilla.org/show_bug.cgi?id=1050447#c32

I wasn't able to observe any problem caused by hiding the autocomplete box. Maybe I'm just too slow a typist. In any case, I just added an extra flag for it.
Attachment #8491545 - Flags: review?(mconley)
Flags: needinfo?(mconley)
Comment on attachment 8491545 [details] [diff] [review]
url-bar-issue

I don't think we want to focus the location bar asynchronously, as the user should be able to start typing immediately after opening a blank tab.
Attachment #8491545 - Flags: feedback-
Let me see if I can come up with an alternative proposal.
Flags: needinfo?(mconley)
(In reply to Dão Gottwald [:dao] from comment #3)
> Comment on attachment 8491545 [details] [diff] [review]
> url-bar-issue
> 
> I don't think we want to focus the location bar asynchronously, as the user
> should be able to start typing immediately after opening a blank tab.

Won't we have a similar problem if the user hits Ctrl-PgUp and immediately starts typing? It's starting to sound like _adjustFocusAfterTabSwitch should happen when we start to switch tabs rather than when we finish.
(In reply to Bill McCloskey (:billm) from comment #5)
> (In reply to Dão Gottwald [:dao] from comment #3)
> > Comment on attachment 8491545 [details] [diff] [review]
> > url-bar-issue
> > 
> > I don't think we want to focus the location bar asynchronously, as the user
> > should be able to start typing immediately after opening a blank tab.
> 
> Won't we have a similar problem if the user hits Ctrl-PgUp and immediately
> starts typing? It's starting to sound like _adjustFocusAfterTabSwitch should
> happen when we start to switch tabs rather than when we finish.

At least for the URL focusing bit, I buy that.
Flags: needinfo?(mconley)
Flags: needinfo?(mconley)
Comment on attachment 8491545 [details] [diff] [review]
url-bar-issue

Canceling review request until I can evaluate some options here.
Attachment #8491545 - Flags: review?(mconley)
Assignee: nobody → mconley
Flags: qe-verify?
Flags: firefox-backlog+
MarcoM asked me to remove my assignee status to see if somebody working with the Firefox backlog might take this bug. If nobody nabs in the next two days, I'll pick it up again.
Assignee: mconley → nobody
Hi Dao, can you mark this bug for QE verification.
Assignee: nobody → dao
Status: NEW → ASSIGNED
Iteration: --- → 35.3
Points: --- → 8
Flags: needinfo?(dao)
Flags: qe-verify?
Flags: qe-verify+
Flags: needinfo?(dao)
Looks like dao's taking the reigns on this one, so clearing my needinfo.
Flags: needinfo?(mconley)
QA Contact: alexandra.lucinet
Any updates here Dao?
Flags: needinfo?(dao)
Attached patch patchSplinter Review
Attachment #8491545 - Attachment is obsolete: true
Attachment #8502507 - Flags: review?(mconley)
Flags: needinfo?(dao)
Keywords: regression
Hardware: x86_64 → All
Summary: [e10s] Location bar occasionally gets focus when it shouldn't → [e10s] Location bar gets focus when switching back to the previous tab after opening a new one
Version: 34 Branch → Trunk
Comment on attachment 8502507 [details] [diff] [review]
patch

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

Looks good dao - I tested the patch and it fixes the problem nicely. Just one nit below.

::: browser/base/content/tabbrowser.xml
@@ +1166,5 @@
>                  //     is the top-most prompt on the tab.
>                  promptBox.removePrompt(prompts[prompts.length - 1]);
>                }
>  
> +              oldBrowser._urlbarFocused = (gURLBar && gURLBar.focused);

Ah, yes, it makes sense to update the oldBrowser's stored state before the switch occurs.

@@ -1185,5 @@
>          <body><![CDATA[
>          let newBrowser = this.getBrowserForTab(newTab);
>          let oldBrowser = this.getBrowserForTab(oldTab);
>  
> -        if (oldBrowser) {

oldBrowser is no longer used in this function, so we can get rid of entirely.
Attachment #8502507 - Flags: review?(mconley) → review+
https://hg.mozilla.org/mozilla-central/rev/1c0aefdb71f9
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
QA Contact: alexandra.lucinet → florin.mezei
Reproduced the initial issue following steps from comment 1 (with e10s enabled), on Win 7 x64, with 35 Nightly (BuildID=20141002093155).

The issue no longer reproduces in latest 35 Nightly (BuildID=20141012030203) on Win 7 x64, Mac OS X 10.9.5 and Ubuntu 13.04 x64. Now, when returning to the first tab (with mouse, Ctrl+PgUp, Ctrl+PgDown) the focus is still on the page content, and not the Location bar as before. This is consistent with non-e10s mode.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.