1691214 - window.focus() does not work after window.open(url, windowName, feature) if the same windowName exists,

Status: VERIFIED FIXED

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

Description

[Alice0775 White]

Attachment: testcase html

window.focus() does not work after window.open(url, windowName, feature) if the same windowName exists,

<button onclick='win=window.open("https://developer.mozilla.org/en-US/", "childWin", "height=300,width=600"); win.focus()'>MDN</button>

<button onclick='win=window.open("https://wiki.mozilla.org/Main_Page", "childWin", "height=300,width=600"); win.focus()'>mozilla wiki</button>

Steps to reproduce:

1. Open attached testcase html
2. Click 1st button
   --- child window pops up at top most window as expected.
3. Focus to the opener
4. Click 2nd button
   --- observe bug

Actual result:
The child window would not be the top most window

Expected results:
The child window should be the top most window

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=ccbeb11eca6fdbb3157af26afc10bd20909f20a5&tochange=c7f363ff3c4fdf8134d24ccc5db0042a4baf949d

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Hmm, I would have expected https://phabricator.services.mozilla.com/D103720 to fix this... Thanks for filing Alice!

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1691214 - Take focus from window.open etc even if we're already active but not in the active window. r=hsivonen

With multiple windows, being active doesn't guarantee being in the
foreground window.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d807fa812d8f
Take focus from window.open etc even if we're already active. r=hsivonen

Comment 4

[Sandor Molnar[:smolnar]]

Backed out for causing failures in browser_bug1691214.

Backout link: https://hg.mozilla.org/integration/autoland/rev/6e5caabf4c1fc44de3afcb2ffa565127d40ebb28

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=ZmBZKcrKTt-9szlVi14UeA.0&resultStatus=testfailed%2Cbusted%2Cexception%2Crunnable&revision=d807fa812d8fd040b0ca860ee5ada95bd258f226

Failure log: https://treeherder.mozilla.org/logviewer?job_id=329205186&repo=autoland&lineNumber=2591

"TEST-PASS | dom/base/test/browser_bug1691214.js | First navigation should've opened the new window -
[task 2021-02-08T11:08:58.367Z] 11:08:58 INFO - Buffered messages finished
[task 2021-02-08T11:08:58.367Z] 11:08:58 INFO - TEST-UNEXPECTED-FAIL | dom/base/test/browser_bug1691214.js | New window should be focused - Got [object Window], expected [object Window]
[task 2021-02-08T11:08:58.367Z] 11:08:58 INFO - Stack trace:
[task 2021-02-08T11:08:58.367Z] 11:08:58 INFO - chrome://mochikit/content/browser-test.js:test_is:1351
[task 2021-02-08T11:08:58.367Z] 11:08:58 INFO - chrome://mochitests/content/browser/dom/base/test/browser_bug1691214.js:null:17
[task 2021-02-08T11:08:58.367Z] 11:08:58 INFO - TEST-PASS | dom/base/test/browser_bug1691214.js | Should've focused initial window back -
[task 2021-02-08T11:08:58.367Z] 11:08:58 INFO - Not taking screenshot here: see the one that was previously logged
[task 2021-02-08T11:08:58.367Z] 11:08:58 INFO - TEST-UNEXPECTED-FAIL | dom/base/test/browser_bug1691214.js | Existing window should've been targeted and focused - Got [object Window], expected [object Window]
[task 2021-02-08T11:08:58.367Z] 11:08:58 INFO - Stack trace:
[task 2021-02-08T11:08:58.367Z] 11:08:58 INFO - chrome://mochikit/content/browser-test.js:test_is:1351
[task 2021-02-08T11:08:58.368Z] 11:08:58 INFO - chrome://mochitests/content/browser/dom/base/test/browser_bug1691214.js:null:31
[task 2021-02-08T11:08:58.368Z] 11:08:58 INFO - Leaving test bound
[task 2021-02-08T11:08:58.415Z] 11:08:58 INFO - GECKO(2408) | MEMORY STAT | vsize 130551186MB | residentFast 2202MB
[task 2021-02-08T11:08:58.415Z] 11:08:58 INFO - TEST-OK | dom/base/test/browser_bug1691214.js | took 2458ms"

Comment 5

[Bogdan Tara[:bogdan_tara | bogdant]]

Also seeing the following starting to perma fail with the backed out changes:
https://treeherder.mozilla.org/logviewer?job_id=329205173&repo=autoland&lineNumber=2358
https://treeherder.mozilla.org/logviewer?job_id=329205182&repo=autoland&lineNumber=2693
https://treeherder.mozilla.org/logviewer?job_id=329215035&repo=autoland&lineNumber=11466
https://treeherder.mozilla.org/logviewer?job_id=329217695&repo=autoland&lineNumber=5349
https://treeherder.mozilla.org/logviewer?job_id=329216310&repo=autoland&lineNumber=2131

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d3539dfe1b07
Take focus from window.open etc even if we're already active but not in the active window. r=hsivonen

Comment 7

[Natalia Csoregi [:nataliaCs]]

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

Comment 8

[Pascal Chevrel:pascalc]

We have shipped our last beta but this was a regression in the 86 cycle, emilio do you think it would be justified and not risky to uplift it in RC or can it ride the 87 train?

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9201660 [details]
Bug 1691214 - Take focus from window.open etc even if we're already active but not in the active window. r=hsivonen

Beta/Release Uplift Approval Request

• User impact if declined: Some apps that use multiple windows may seem like not working.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: comment 0
• List of other uplifts needed: none
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): I think it's relatively low-risk since this is a minor extension of bug 1690099 to apply to the multi-window case, and it's a relatively well-tested code-path in general for the cases where the special-case shouldn't apply.

This showed up in enterprise applications (https://bugzilla.mozilla.org/show_bug.cgi?id=1691204#c2), so it might be worth fixing.

• String changes made/needed: none

Comment 11

[Pascal Chevrel:pascalc]

Comment on attachment 9201660 [details]
Bug 1691214 - Take focus from window.open etc even if we're already active but not in the active window. r=hsivonen

Let's take it for our RC as this is an S2 and an 86 regression with already a duplicate report in a corporate environment. The patch has tests and emilio is is confident that it should be low risk. Thanks!

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d76bfb0d467f

Comment 13

[Peter Magyari (Desktop QA)]

Reproduced the issue on Nightly 87.0a1 (20210206094243) and Beta 86.0b7 (20210207185447)
Verified the fix on Nightly 87.0a1 (20210210215051) and Beta 86.0b7 (20210212204714)