Closed Bug 1336445 Opened 3 years ago Closed 3 years ago

If switch_to_window is called with a chrome handle, the first tab of that window gets selected

Categories

(Testing :: Marionette, defect)

Version 3
defect
Not set

Tracking

(firefox52 fixed, firefox53 fixed, firefox54 fixed)

RESOLVED FIXED
mozilla54
Tracking Status
firefox52 --- fixed
firefox53 --- fixed
firefox54 --- fixed

People

(Reporter: whimboo, Assigned: whimboo)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

We have some broken behavior in our switchToWindow() implementation which causes us to select the first tab when actually switching to the chrome window handle. This is because we also take all the content window handles into account, and return the index=0 for the first match.

We should only check the content windows if the handle is not for a chrome window.

Patch and test is upcoming.

This blocks bug 1322383 from landing, because it found this issue.
Attachment #8833325 - Flags: review?(ato)
Comment on attachment 8833325 [details]
Bug 1336445 - Don't select the first tab if switch_to_window() is called with a chrome window handle.

https://reviewboard.mozilla.org/r/109584/#review110662
Attachment #8833325 - Flags: review?(ato) → review+
I had a look and totally my fault! When I see this I really wonder why no other Mn test actually noticed that problem! What we miss here is to initialize the first content browser when opening a new chrome window. Because we look for tabIndex, which will no longer be set by moving those code lines above the other block. 

I have a working solution locally but will add another unit test.
Flags: needinfo?(hskupin)
The code path you actually have to take to reproduce this problem is opening a new private browsing window. If you don't do that it's not reproducible. At least I can setup a proper unit test for marionette now.
Attachment #8833325 - Flags: review+ → review?(ato)
Comment on attachment 8833325 [details]
Bug 1336445 - Don't select the first tab if switch_to_window() is called with a chrome window handle.

Try shows bustage somewhere in session creation for fx-ui-tests.
Attachment #8833325 - Flags: review?(ato)
Comment on attachment 8833325 [details]
Bug 1336445 - Don't select the first tab if switch_to_window() is called with a chrome window handle.

Silly mistake I made in the last revision. The property hasTabBrowser is always present for non-browser windows. So a check for `"hasTabBrowser" in found` always results in trying to register browsers, which caused an obvious hang.
Attachment #8833325 - Flags: review?(ato)
Last update contains a small fix for a race condition I noticed in the try builds on Linux, where we do not wait long enough for the body element to appear. It's better to exchange it with a call to execute_script() which indeed has the same effect.
Comment on attachment 8833325 [details]
Bug 1336445 - Don't select the first tab if switch_to_window() is called with a chrome window handle.

https://reviewboard.mozilla.org/r/109584/#review111100
Attachment #8833325 - Flags: review?(ato) → review+
The latest revision looks excellent.  It would be good if we could uplift this to the relevant channels, if possible.
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f08c78b5f19b
Don't select the first tab if switch_to_window() is called with a chrome window handle. r=ato
https://hg.mozilla.org/mozilla-central/rev/f08c78b5f19b
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
test-only changes we would like to see uplifted for aurora and beta. Thanks.
Whiteboard: [checkin-needed-aurora][checkin-needed-beta]
https://hg.mozilla.org/releases/mozilla-aurora/rev/4116e3e12460
Whiteboard: [checkin-needed-aurora][checkin-needed-beta] → [checkin-needed-beta]
https://hg.mozilla.org/releases/mozilla-beta/rev/335f8a267be8
Flags: in-testsuite+
Whiteboard: [checkin-needed-beta]
You need to log in before you can comment on or make changes to this bug.