Skip to content
This repository has been archived by the owner on Feb 29, 2020. It is now read-only.

Perma-orange: browser_bug{455852,520538}.js location bar is focused for the new tab #2698

Closed
Mardak opened this issue Jun 10, 2017 · 10 comments

Comments

@Mardak
Copy link
Member

Mardak commented Jun 10, 2017

Also see Bugzilla/PR https://bugzilla.mozilla.org/show_bug.cgi?id=1374768

These similar tests have been consistently failing:

TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_bug455852.js | location bar is focused for the new tab - Got [object XULElement], expected [object HTMLInputElement] 

https://treeherder.mozilla.org/logviewer.html#?job_id=106084575&repo=pine&lineNumber=3619


TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_bug520538.js | '--new-tab about:blank' focuses the location bar - Got [object XULElement], expected [object HTMLInputElement]

https://treeherder.mozilla.org/logviewer.html#?job_id=106084575&repo=pine&lineNumber=4152

@Mardak
Copy link
Member Author

Mardak commented Jun 15, 2017

Bisected this regression to mconley's fix for https://bugzilla.mozilla.org/show_bug.cgi?id=1367596
https://hg.mozilla.org/mozilla-central/rev/fb99ac0fe4e3

The [object XULElement] that has focus seems to be the xul:browser instead of the location bar on closing the last tab of the window when browser.tabs.closeWindowWithLastTab is false.

@k88hudson k88hudson added the Fx55 label Jun 19, 2017
@sarracini sarracini added this to Assigned / Milestone 0 in Land in Nightly / Graduate Jun 19, 2017
@sarracini sarracini moved this from Assigned / Milestone 0 to Milestone 1 in Land in Nightly / Graduate Jun 19, 2017
@dmose dmose self-assigned this Jun 20, 2017
@dmose
Copy link
Member

dmose commented Jun 20, 2017

@Mardak I'm not seeing this failure locally at all. Is there anything specific you needed to do to reproduce it?

@dmose
Copy link
Member

dmose commented Jun 20, 2017

Never mind; I see now that this is Linux-only failure. Installing VM...

@dmose
Copy link
Member

dmose commented Jun 20, 2017

And looking more closely at pine, I see that it is failing on OS X debug too. Hmmm...

@Mardak
Copy link
Member Author

Mardak commented Jun 20, 2017

Yeah, the latest pine push has it failing on mac as well. I'm trying to see what I did before when bisecting. Pretty sure I needed to run the two tests together as one would set up the focus and the second would maintain the broken focus (instead of moving).

https://treeherder.mozilla.org/logviewer.html#?job_id=108589851&repo=pine&lineNumber=5429

./mach mochitest --setpref browser.newtabpage.activity-stream.enabled=true browser/base/content/test/general/browser_bug{455852,520538}.js

However, that isn't failing for me right now…

@Mardak
Copy link
Member Author

Mardak commented Jun 20, 2017

Aha! This is what I actually used. It happens to pull in one other test that moves focus before the 455852 test runs and fails.

./mach mochitest --setpref browser.newtabpage.activity-stream.enabled=true browser/base/content/test/general/browser_bug4*2.js

@Mardak
Copy link
Member Author

Mardak commented Jun 20, 2017

@Mardak Mardak assigned Mardak and unassigned dmose Jun 20, 2017
@Mardak
Copy link
Member Author

Mardak commented Jun 20, 2017

Notes from mconley about the regressing change:

So this is part of a tab switch optimization where we feel like in certain cases, it's appropriate / acceptable to switch to out-of-process browsers synchronously, and show a blank white content area.
The typical example being a tab that has just opened and started (but not yet finished) loading a thing
It is possible, however, to load a tab in the background, wait for a while before seeing it, having it finish loading, and then switching to it. I do that all of the time when browsing r/aww on Reddit. Just a ton of cute animal GIFs and images in background tabs waiting for me.
In those cases, we judge in a very rough way that a tab has "sufficiently loaded" when the throbber is no longer being displayed for it at that point, the tab switcher decides that we can't take the "blank" shortcut I described earlier, and does an async tab switch, to avoid a flash of white

If shouldBeBlank is false, then yes, we'll attempt an asynchronous tab switch

add a new method to tabbrowser that takes in a URI and says whether or not it's something that'd show progress
so both mTabProgressListener and the async tab switcher can call it
then inside updateDisplay, have it pass the URI and use that to determine if we're "sufficiently loaded"

@Mardak
Copy link
Member Author

Mardak commented Jun 27, 2017

Will need to be addressed later now that content process is being backed out in #2756 but will need to be fixed to turn it back on for #2777.

@Mardak Mardak changed the title Perma-orange: browser_bug455852.js and browser_bug520538.js Perma-orange: browser_bug{455852,520538}.js location bar is focused for the new tab Jun 29, 2017
@Mardak Mardak added this to All Platfoms in Pref-on Test Failures Jun 30, 2017
@Mardak Mardak closed this as completed Jul 7, 2017
@Mardak Mardak moved this from All Platfoms to Done in Pref-on Test Failures Jul 7, 2017
@sarracini sarracini moved this from Assigned / Milestone 1 to Complete in Land in Nightly / Graduate Jul 10, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

No branches or pull requests

5 participants