1358926 - Location bar non-functional after closing dialog such as "Clear All History" and "Properties for bookmark"

Status: VERIFIED FIXED

(Core :: DOM: UI Events & Focus Handling, defect, P1)

Description

[u474838]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:53.0) Gecko/20100101 Firefox/53.0
Build ID: 20170414022702

Steps to reproduce:

1. Configure browser.tabs.closeWindowWithLastTab=false
2. Restart
3. Browse a little bit with onr or more tabs
4. Close all tabs, being left on a single, empty tab with no content
5. Use Ctrl-Alt-Delete to clear all history
6. Now the tab is non-function and entering any URL in the address bar and trying to navigate to results in a NOP

If on the single and last tab open you don't close the page with Ctrl-W and have still content, clearing all history doesn't make the tab non-functional.

To recover, one has to close the non-functional tab to get a fresh, empty and functioning tab.


Actual results:

Tab died unexpectedly without any signal that it's now non-functional.


Expected results:

Tab should have been alive without content as it did in 52.

Comment 1

[u474838]

The best word for it would be that it's a visible but dead tab or zombie tab.

Comment 2

[:Gijs (he/him)]

It would be useful if someone found out when exactly this broke between 52 and 53, e.g. using mozregression: https://mozilla.github.io/mozregression/ .

Comment 3

[u474838]

If I open Bookmarks with Ctrl-Shift-o and close it, then the empty and non-functional tab is suddenly alive again.

Comment 4

[u474838]

(In reply to :Gijs from comment #2)
> It would be useful if someone found out when exactly this broke between 52
> and 53, e.g. using mozregression: https://mozilla.github.io/mozregression/ .

I'm quite certain this didn't happen in 52.0.2 but I haven't tested it since the 53 update.

Comment 5

[u474838]

It seems it's not that the tab is completely zombie since changing focus from the address bar to the page view and back also revives the dead address bar.

Comment 6

[:Gijs (he/him)]

I can't reproduce this at all on OS X. Either this is Linux-specific or there's something else going on.

(In reply to Carsten Mattner from comment #0)
> User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:53.0) Gecko/20100101
> Firefox/53.0
> Build ID: 20170414022702
> 
> Steps to reproduce:
> 
> 1. Configure browser.tabs.closeWindowWithLastTab=false
> 2. Restart
> 3. Browse a little bit with onr or more tabs
> 4. Close all tabs, being left on a single, empty tab with no content

I see the default new tab page. Tested on a clean profile. Are you testing on a profile with add-ons or other custom settings, maybe?

Would it be possible for you to run mozregression to find out exactly when/how this broke?

Comment 7

[u474838]

(In reply to :Gijs from comment #6)
> I can't reproduce this at all on OS X. Either this is Linux-specific or
> there's something else going on.

It's probably GTK3 since the address bar looks like a plain old gtk
text control to me.

Once you have a single, empty tab, you clear all history, and focus
of a new tab has already been the address bar. Switching focus
around revises the address bar.

> (In reply to Carsten Mattner from comment #0)
> > User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:53.0) Gecko/20100101
> > Firefox/53.0
> > Build ID: 20170414022702
> > 
> > Steps to reproduce:
> > 
> > 1. Configure browser.tabs.closeWindowWithLastTab=false
> > 2. Restart
> > 3. Browse a little bit with onr or more tabs
> > 4. Close all tabs, being left on a single, empty tab with no content
> 
> I see the default new tab page. Tested on a clean profile. Are you testing
> on a profile with add-ons or other custom settings, maybe?
> 
> Would it be possible for you to run mozregression to find out exactly
> when/how this broke?

Do you know the presumed good build date (52.0.2)? If mozregression
from mozregression-gui-0.9.7.tar.gz works out of the box, I might try
it. If it takes effort to get going, I might not be able to invest the
needed time. Does mozregression just fetch the regular x86_64 Linux
builds from ftp.mozilla.org? Not sure when, though.

Comment 8

[:Gijs (he/him)]

(In reply to Carsten Mattner from comment #7)
> > (In reply to Carsten Mattner from comment #0)
> > > User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:53.0) Gecko/20100101
> > > Firefox/53.0
> > > Build ID: 20170414022702
> > > 
> > > Steps to reproduce:
> > > 
> > > 1. Configure browser.tabs.closeWindowWithLastTab=false
> > > 2. Restart
> > > 3. Browse a little bit with onr or more tabs
> > > 4. Close all tabs, being left on a single, empty tab with no content
> > 
> > I see the default new tab page. Tested on a clean profile. Are you testing
> > on a profile with add-ons or other custom settings, maybe?
> > 
> > Would it be possible for you to run mozregression to find out exactly
> > when/how this broke?
> 
> Do you know the presumed good build date (52.0.2)?

You can just use a release number with mozregression on the commandline (--good 52). Not sure about the gui. Either way, it'll test nightly builds with a clean profile (unless otherwise configured), not touching your 'regular' setup.

> If mozregression
> from mozregression-gui-0.9.7.tar.gz works out of the box, I might try
> it. If it takes effort to get going, I might not be able to invest the
> needed time. Does mozregression just fetch the regular x86_64 Linux
> builds from ftp.mozilla.org? Not sure when, though.

It'll fetch the appropriate linux nightly builds, yes. I would advise to not put in a specific "bad" date as any changes might have been uplifted, and so the nightly regression range might be after 53 branched from nightly to aurora/beta. Because it does a binary search with you, the number of builds it'll download will be relatively small and the whole process should be pretty quick assuming a good internet connection.

Comment 9

[Stefan [:StefanG_QA]]

Mozilla/5.0 (X11; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0 

I've tested the reported issue on Ubuntu 16.04 with latest Firefox release and latest Nightly build (20170426100344) and I was able to reproduce it with the provided steps from comment 0. 

I've used mozregression in order to find a regression range. Here are my findings: 

Last good revision: a8d497b09753c91783b68c5805c64f34a2f39629
First bad revision: 4ceb9062ea8f4113bfd1b3536ace4a840a72faa7
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a8d497b09753c91783b68c5805c64f34a2f39629&tochange=4ceb9062ea8f4113bfd1b3536ace4a840a72faa7

Comment 10

[:Gijs (he/him)]

(In reply to Stefan [:StefanG_QA] from comment #9)
> Mozilla/5.0 (X11; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0 
> 
> I've tested the reported issue on Ubuntu 16.04 with latest Firefox release
> and latest Nightly build (20170426100344) and I was able to reproduce it
> with the provided steps from comment 0. 
> 
> I've used mozregression in order to find a regression range. Here are my
> findings: 
> 
> Last good revision: a8d497b09753c91783b68c5805c64f34a2f39629
> First bad revision: 4ceb9062ea8f4113bfd1b3536ace4a840a72faa7
> https://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=a8d497b09753c91783b68c5805c64f34a2f39629&tochange=4ceb
> 9062ea8f4113bfd1b3536ace4a840a72faa7

Can you get a more regression range from autoland?

Comment 11

[:Gijs (he/him)]

(In reply to :Gijs from comment #10)
> Can you get a more regression range from autoland?

Err, a more *detailed* regression range (ie, narrower)?

Comment 12

[Alice0775 White]

I have a defferent regression range.

Reproducible : 100%

Steps to reproduce:

1. Start Firefox with newly created profile and close default browser popup window if any
2. Wait to loading First Run Nightly page
3. Open New Tab with click [+] button
4. Use Ctrl+Shift+Del to clear all history and click [Clear Now] button
5. Enter something in location bar and Hit ENTER

AR:
No suggestion dropdown list pops up when type something.
Nothing happens after hit ENTER key.

It will work again after switch between tabs.

ER:
Suggestion dropdown list should be popup when type something.
Page should loads after hit ENTER key.
The location bar should be ready to function immediately after "Clear All History" closed.

Regression window:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=671a2f8542cadcf9aad318ab76250a730a03b240&tochange=ac3275723df59db0f09198fdb61b51e7002c391a

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=e8b32aaa2eec84c15f44cfd677142a0047481152&tochange=f3ccf0f50160e169389aa26a2d95ec225effcd3b

Suspect: Bug 1316330

@:jessica
It seems that your patch caused problems

Comment 13

[Alice0775 White]

No need actually clear history, jusu cancelling is enough.

4. Use Ctrl+Shift+Del to open dialog "Clear All History" and click [Cancel] button


So, it seems focus related bug.

Comment 14

[Jessica Jong [:jessica] (inactive)]

I can reproduce it too using the STR in comment 12, but only on Ubuntu, not on Mac.
It seems that the focus does not get back to the location bar. I'll take a look, but I may need a little help since I'm not sure how to debug the upper part (non-content) of the browser...

Comment 15

[Jessica Jong [:jessica] (inactive)]

When this issue happens, the order of the events received after dismissing the dialog is:
- focus
- blur
so, the url bar is not focused.
Hmm, I thought I fixed this in the followup patch in bug 1316330. Let me dig deeper..

Comment 16

[Alice0775 White]

"Properties for bookmark" also triggers the problem

1. Open New Tab with click [+] button
2. Right click on a Bookmark, and choose "Properties" to open properties dialog and Click [Cancel] button
3. Enter something in location bar and Hit ENTER

The problem seems to be serious for user experience.

Comment 17

[u474838]

Thanks everyone for bisecting before I managed to get around to it.

Comment 18

[u474838]

(In reply to Jessica Jong [:jessica] from comment #14)
> but I may need a little help since I'm not sure how to debug the upper
> part (non-content) of the browser...

Hi Jessica, in case it's unclear. My initial observation that the
whole tab was a zombie turned out to be false, as mentioned in my
follow-up observations. After noticing that a simple focus
switcheroo from location to search control and back revives the
local control, it became clear there's a focus bug. Still it is
interesting that having a non-empty but also single tab is not
affected.

Comment 19

[u474838]

Switcheroo remedy: Ctrl-k and then Ctrl-l

Comment 20

[Liz Henry (:lizzard) (relman/hg->git project)]

Too late to fix in 53. If you disagree, please let me know - for example if the problem turns out to affect a lot of users. 
We could likely still take a patch in 54.  
Jessica, are you taking on this bug? Maybe smaug can help.

Comment 21

[Jessica Jong [:jessica] (inactive)]

I agree on a wontfix in 53. This may happen after opening a modal dialog, but it's timing related.

Looking at this closer...
When the modal dialog shows, we add the urlbar blur event to the `mDelayedBlurFocusEvents` queue. When the modal dialog is dismissed, we dispatch `nsDelayedEventDispatcher` in `nsDocument::UnsuppressEventHandlingAndFireEvents`, but before the delayed event is fired, the urlbar focus event is fired directly. So, the delayed blur event comes after the focus event.

One way to fix this is, before firing events, we check if `mDelayedBlurFocusEvents` is empty or not, if it's not empty, we fire the events in `mDelayedBlurFocusEvents` first.

Hopefully I can have a patch by today and see what smaug thinks about it.

Comment 22

[Jessica Jong [:jessica] (inactive)]

Attachment: patch, v1.

When firing an event, we check for the delayed event queue first, if there are events in the queue that belong to the same doc, fire them first.

We already have a similar test for it [1], but I'm not sure how to make the blur event delayed but the focus event not delayed, like in this case.

[1] http://searchfox.org/mozilla-central/source/dom/tests/browser/browser_test_focus_after_modal_state.js

Comment 23

[Jessica Jong [:jessica] (inactive)]

Patch is ready, I'll ask for review once smaug's review queue is open. :)

Comment 24

[u474838]

I had another disabled location bar today when there were three tabs open, but I cannot reproduce it since I don't know the steps it took. Jessica, is it possible the bug would cause that as well and it's not limited to one tab per window scenarios?

Comment 25

[u474838]

I wanted to test the patch on top of 53.0.2 hg tip after importing it,
but since mach picks up my global rustc which happens to be at 1.17.0
I suspect it fails when building the Rust code (mp4 container parser).

I can build and test the patch if someone can hint me at the right way
to fix this:

 1:33.78 error[E0463]: can't find crate for `cheddar`
 1:33.78  --> /tmp/mozilla/firefox/media/libstagefright/binding/mp4parse_capi/build.rs:1:1
 1:33.78   |
 1:33.78 1 | extern crate cheddar;
 1:33.79   | ^^^^^^^^^^^^^^^^^^^^^ can't find crate
 1:33.79
 1:33.79 error: aborting due to previous error
 1:33.79
 1:33.80 error: Could not compile `mp4parse_capi`.

Does it require rustc <= 1.16 or why doesn't Firefox (mach) simply get the right
rust toolchain it is compatible with?

Comment 26

[Jessica Jong [:jessica] (inactive)]

(In reply to Carsten Mattner from comment #24)
> I had another disabled location bar today when there were three tabs open,
> but I cannot reproduce it since I don't know the steps it took. Jessica, is
> it possible the bug would cause that as well and it's not limited to one tab
> per window scenarios?

Yes, I think it's not limited to one tab per window scenarios. It's all about timing.
 
> Does it require rustc <= 1.16 or why doesn't Firefox (mach) simply get the
> right
> rust toolchain it is compatible with?

I didn't have this problem. Were you able to build Firefox successfully before? Maybe you can try running `./mach bootstrap` again. Anyways, I'm going to mark checkin-needed now, so you may be able to test it on Nightly soon. Thanks for helping. :)

Comment 27

[Jessica Jong [:jessica] (inactive)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=455ff17f27ae134d1d70b59e4548b5c7858d8474&group_state=expanded

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/054c35905152
Check and dequeue delayed events in the queue before firing events. r=smaug

Comment 29

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/054c35905152

Comment 30

[Nathan Froyd [:froydnj]]

Is this appropriate for uplift to 54?  If so, can you please file the uplift request?  Thanks!

Comment 31

[Jessica Jong [:jessica] (inactive)]

(In reply to Nathan Froyd [:froydnj] from comment #30)
> Is this appropriate for uplift to 54?  If so, can you please file the uplift
> request?  Thanks!

Sure, but I'd like to make sure this has been fixed first. Keeping the ni for tracking.

Carsten, is it possible for you to verify this on Nightly? Thanks.

Comment 32

[u474838]

Confirmed resolved in x86_64 Linux GTK3 nightly built from https://hg.mozilla.org/mozilla-central/rev/82c2d17e74ef9cdf38a5d5ac4eb3ae846ec30ba4

Comment 33

[u474838]

While verifying the fix and taking a close look at the location bar, I noticed that 55 nightly differs from 53 stable.

Say you go to youtube.com, load some channel, then enter the location bar to change the CHANNEL part of /user/CHANNEL/videos. In 55 nightly there's flashing/flickering of the site's icon while seemingly more aggressively presenting predictive results to navigate to in the dropdown list. Is this new in 55 or a side effect of the fix's event firing changes?

Comment 34

[Jessica Jong [:jessica] (inactive)]

(In reply to Carsten Mattner from comment #33)
> While verifying the fix and taking a close look at the location bar, I
> noticed that 55 nightly differs from 53 stable.
> 
> Say you go to youtube.com, load some channel, then enter the location bar to
> change the CHANNEL part of /user/CHANNEL/videos. In 55 nightly there's
> flashing/flickering of the site's icon while seemingly more aggressively
> presenting predictive results to navigate to in the dropdown list. Is this
> new in 55 or a side effect of the fix's event firing changes?

Thanks for verifying.
I'm not sure about the flickering, but the fix should only affect delayed events (events that are suppressed/delayed due to modal dialog or sync xhr).

Comment 35

[Jessica Jong [:jessica] (inactive)]

Comment on attachment 8862775 [details] [diff] [review]
patch, v1.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1316330
[User impact if declined]: focus not working properly after modal dialog, which causes unexpected behavior, e.g. dead url bar
[Is this code covered by automated tests?]: not this exact scenario, it's hard to produce the exact scenario in automated tests
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: low-risk
[Why is the change risky/not risky?]: fire delayed events first, which make them in the right order
[String changes made/needed]: none

Comment 36

[Gerry Chang [:gchang]]

Comment on attachment 8862775 [details] [diff] [review]
patch, v1.

Fix a location bar regression issue and was verified. Beta54+. Should be in 54 beta 6.

Comment 37

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/4844f1db8655240f9b7d9fc400539b1dfb80b915

Comment 38

[Paul Silaghi, QA [:pauly]]

Reproduced on Fx 53.0.2, Ubuntu 16.04.
Verified fixed Fx 54b6, 55.0a1 (2017-05-09).