Henrik Skupin [:whimboo][⌚️UTC+1] Assignee
	Description • 7 years ago

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.

	Andreas Tolfsen ❲:ato❳
	Comment 2 • 7 years ago
mozreview-review

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

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 3 • 7 years ago

https://hg.mozilla.org/integration/autoland/rev/e9bc1d6fadb4a61c469e3d8d9a0afc8eda7c92f0

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 4 • 7 years ago

Backed out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=74406761&repo=autoland

https://hg.mozilla.org/integration/autoland/rev/711aa14e4c4cfe5a39fd3c06ac67c4bb31573375

	Henrik Skupin [:whimboo][⌚️UTC+1] Assignee
	Comment 5 • 7 years ago

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.

	Henrik Skupin [:whimboo][⌚️UTC+1] Assignee
	Comment 6 • 7 years ago

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.

	Henrik Skupin [:whimboo][⌚️UTC+1] Assignee
	Comment 8 • 7 years ago

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.

	Henrik Skupin [:whimboo][⌚️UTC+1] Assignee
	Comment 10 • 7 years ago

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.

	Henrik Skupin [:whimboo][⌚️UTC+1] Assignee
	Comment 13 • 7 years ago

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.

	Andreas Tolfsen ❲:ato❳
	Comment 14 • 7 years ago
mozreview-review

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

	Andreas Tolfsen ❲:ato❳
	Comment 15 • 7 years ago

The latest revision looks excellent.  It would be good if we could uplift this to the relevant channels, if possible.

	Pulsebot
	Comment 16 • 7 years ago

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

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 17 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/f08c78b5f19b

	Henrik Skupin [:whimboo][⌚️UTC+1] Assignee
	Comment 18 • 7 years ago

test-only changes we would like to see uplifted for aurora and beta. Thanks.

	Carsten Book [:Tomcat]
	Comment 19 • 7 years ago
bugherder uplift

https://hg.mozilla.org/releases/mozilla-aurora/rev/4116e3e12460

	Ryan VanderMeulen [:RyanVM]
	Comment 20 • 7 years ago
bugherder uplift

https://hg.mozilla.org/releases/mozilla-beta/rev/335f8a267be8