1686989 - Inconsistent focus state when opening multiple SubDialogs via SubDialogManager

Status: VERIFIED FIXED

(Toolkit Graveyard :: Notifications and Alerts, defect)

Description

[Emma Zühlcke [:emz]]

Spawning multiple SubDialogs in a stack managed by the SubDialogManager can lead to inconsistent focus state, where dialogs get focused even when they're not on top.

Comment 1

[Emma Zühlcke [:emz]]

Attachment: Bug 1686989 - Fixed SubDialogManager focus issues and added support for custom focus handlers. r=gijs!

• Content embedded by SubDialog can define its own focus handler which will be called on SubDialog#focus
• Updated CommonDialog to register custom focus handler
• Moved focus calls from SubDialog to SubDialogManager
• Return early after tab dialog focus on tab switch

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/81794f8a220b
Fixed SubDialogManager focus issues and added support for custom focus handlers. r=Gijs

Comment 3

[Natalia Csoregi [:nataliaCs]]

Backed out for auth and dialog box failures.

backout: https://hg.mozilla.org/integration/autoland/rev/35d4ab75c406e70cbec79b2ca00fd347ae843fdc

push: https://treeherder.mozilla.org/jobs?repo=autoland&revision=111af4c2bf6bfad9a928fe8fb25c9bc480da50be&group_state=expanded

failure logs:

• TEST-UNEXPECTED-FAIL | toolkit/components/passwordmgr/test/browser/browser_basicAuth_multiTab.js | Uncaught exception - at resource://testing-common/PromptTestUtils.jsm:175 - TypeError: can't access property "_dialogs", dialogBox._dialogManager is undefined
• TEST-UNEXPECTED-FAIL | browser/base/content/test/tabPrompts/browser_confirmFolderUpload.js | Uncaught exception - at resource://testing-common/PromptTestUtils.jsm:175 - TypeError: can't access property "_dialogs", dialogBox._dialogManager is undefined
• TEST-UNEXPECTED-FAIL | browser/base/content/test/tabdialogs/browser_tabdialogbox_focus.js | Uncaught exception - at chrome://mochitests/content/browser/browser/base/content/test/tabdialogs/browser_tabdialogbox_focus.js:100 - TypeError: can't access property "_dialogs", dialogBox._dialogManager is undefined

Comment 4

[Emma Zühlcke [:emz]]

This failed because Bug 1680637 landed in the meantime which changed the name of the dialogManager property. I'll fix that and run it on try to be sure.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d96ac6e7874e
Fixed SubDialogManager focus issues and added support for custom focus handlers. r=Gijs

Comment 6

[Sandor Molnar[:smolnar]]

Backed out for causing mochitest failures in browser_basicAuth_multiTab.

Backout link: https://hg.mozilla.org/integration/autoland/rev/2ac3e28717211455a5f1e058bba8f70f84fd09ad

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Crunning%2Cpending%2Crunnable&searchStr=mochi%2Ce10s&revision=c2d9108fee938f4ce0031f47ce2195895b6a898a&selectedTaskRun=UBb2e_d3QCimoC2jwSadnw.0

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

:INFO - GECKO(2835) | [Parent 2835, Main Thread] WARNING: Extra shutdown CC: 'i < NORMAL_SHUTDOWN_COLLECTIONS', file /builds/worker/checkouts/gecko/xpcom/base/nsCycleCollector.cpp:3364
[task 2021-01-26T13:24:58.635Z] 13:24:58 INFO - GECKO(2835) | [Parent 2835, Main Thread] WARNING: NS_ENSURE_TRUE(Preferences::InitStaticMembers()) failed: file /builds/worker/checkouts/gecko/modules/libpref/Preferences.cpp:4378
[task 2021-01-26T13:24:58.691Z] 13:24:58 INFO - TEST-INFO | Main app process: exit 0
[task 2021-01-26T13:24:58.692Z] 13:24:58 INFO - TEST-INFO | Confirming we saw 366 DOCSHELL created and 366 destroyed log strings.
[task 2021-01-26T13:24:58.693Z] 13:24:58 INFO - TEST-INFO | Confirming we saw 1071 DOMWINDOW created and 1071 destroyed log strings.
[task 2021-01-26T13:24:58.693Z] 13:24:58 ERROR - TEST-UNEXPECTED-FAIL | toolkit/components/passwordmgr/test/browser/browser_basicAuth_multiTab.js | leaked 2 window(s) until shutdown [url = about:blank]
[task 2021-01-26T13:24:58.694Z] 13:24:58 INFO - TEST-INFO | toolkit/components/passwordmgr/test/browser/browser_basicAuth_multiTab.js | windows(s) leaked: [pid = 2835] [serial = 44], [pid = 2835] [serial = 53]
[task 2021-01-26T13:24:58.694Z] 13:24:58 INFO - TEST-INFO | toolkit/components/passwordmgr/test/browser/browser_basicAuth_multiTab.js | This test created 1 hidden window(s):

Comment 7

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/5ca936ea4848
Backed out 2 changesets (bug 1686989, bug 1684469) for causing mochitest failures in browser_basicAuth_multiTab. CLOSED TREE

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cc60105589f5
Fixed SubDialogManager focus issues and added support for custom focus handlers. r=Gijs

Comment 9

[Andreea Pavel [:apavel]]

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

Comment 10

[Rares Doghi, Desktop QA]

Hi Paul, are there any steps we can use in order to verify this issue ?

Comment 11

[Emma Zühlcke [:emz]]

You can test this with the Browser Toolbox:

1. Open the Browser Toolbox console
2. Spawn 3 prompts at the same time using the following command:

Services.prompt.asyncAlert(gBrowser.selectedBrowser.browsingContext, Services.prompt.MODAL_TYPE_TAB, "Title", "a")
Services.prompt.asyncAlert(gBrowser.selectedBrowser.browsingContext, Services.prompt.MODAL_TYPE_TAB, "Title", "b")
Services.prompt.asyncAlert(gBrowser.selectedBrowser.browsingContext, Services.prompt.MODAL_TYPE_TAB, "Title", "c")

3. Focus the main window
4. Close the prompts by pressing enter 3 times.

Expected:
Every time you press the enter key the top most prompt should close. After pressing the enter key 3 times all prompts should be closed.

Bug:
Not all prompts close because the focus is not always on the top most prompt. The prompts don't close in the correct order.

Comment 12

[Rares Doghi, Desktop QA]

Thanks a lot for the Steps Paul, This issue is Verified as fixed in Firefox 87, as well as our latest Release, Beta and Nightly builds on Windows, Mac and Ubuntu.