1754759 - Detaching a tab into a new window while a modal prompt (alert/prompt/confirm) is up breaks interaction with the tab

Status: RESOLVED FIXED

(Toolkit :: Content Prompts, defect)

Description

[sycxyc+mozilla]

Attachment: testcase_alert.html

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Firefox/91.0

Steps to reproduce:

On Linux Firefox 91.5.0esr or Windows Firefox Developer Edition 98.0b2:

1. Have a window open with at least two tabs.
2. Open alert box on a tab (e.g. html attachment).
3. Move tab to new window.

On Android Firefox 81.1.4:

1. Open alert box on tab (e.g. html attachment).
2. Switch browser to background.
3. Switch browser to foreground.

Actual results:

The alert box is closed abnormally and the page can't interactive (the link in the html attachment is not clickable).

Expected results:

The alert box should not close automatically, or at least not break the interactive input on the page.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Widget: Gtk' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 2

[Alice0775 White]

Regression window(m-c):
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=cf6956a5ec8e21896736f96237b1476c9d0aaf45&tochange=0f5e4a3c6f0a6ab0fcd77c46cedc819e2bd0b3f8

Regression window(autoland cached):
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=3598f96a099133837aa78b6c018ff74ca522e36b&tochange=c5654880057a9011c0d743322b11d2d115ea5fae

From above window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=3598f96a099133837aa78b6c018ff74ca522e36b&tochange=0f5e4a3c6f0a6ab0fcd77c46cedc819e2bd0b3f8

Suspect:
5f472f876f794d3b6be4641bec56586f1b31d319 Micah Tigley — Bug 1680637 - Update tests using the old dialog UI to only use the old UI. r=jaws,Gijs,marionette-reviewers,whimboo,remote-protocol-reviewers
0cf8c2169a0d8cad1e14c52bb97c5cea8764ddb6 Micah Tigley — Bug 1682395 - Center content prompts managed by TabDialogBox r=dao
996fd993849c7343c251f9d183be22c778c531a4 Micah Tigley — Bug 1680637 - Rename TabDialogBox's manager to tabDialogManager r=marionette-reviewers,Gijs
4788e6b14ff41d53ef3cb6dad51118d632589d24 Micah Tigley — Bug 1680637 - Add a dialog manager for content prompts in TabDialogBox r=jaws,Gijs

Micah Tigley, your buntch of patches seems to cause the regression. Could you please look into this?

Comment 3

[Alice0775 White]

[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]: UX get stuck after detach tab

Comment 4

[:Gijs (out for now; he/him)]

FWIW, me clearing the flags was the result of a mid-air, but I don't think this is severe enough to track any release. Detaching tabs isn't a super common workflow, and the trivial workaround is to close the prompt/modal before detaching. Or, if you forget and things end up broken, reloading the page fixes it.

For comparison, detaching a tab breaks copying text in that tab (even without a modal prompt being up!) and that has been broken for more than a year: bug 1681712, and isn't tracking anything.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

:alice0775, since this bug is a regression, could you fill (if possible) the regressed_by field?
For more information, please visit auto_nag documentation.

Comment 6

[:Gijs (out for now; he/him)]

(In reply to sycxyc+mozilla from comment #0)

On Android Firefox 81.1.4:

1. Open alert box on tab (e.g. html attachment).
2. Switch browser to background.
3. Switch browser to foreground.

Sorry, I've just realized this was here - please could you file a separate github ticket at https://github.com/mozilla-mobile/fenix ? The implementation of alert() is not the same between mobile and desktop and so the fenix team will need to look at this separately. Thank you!

Comment 7

[:Gijs (out for now; he/him)]

As a note, before the regression, dragging out the tab would also close the alert/prompt dialog, so the main regression is that we're leaving the page in a broken state. Although it would be nice to have another dialog in the new window, for various technical reasons to do with nested event loops, that probably isn't straightforward.

Comment 8

[sycxyc+mozilla]

(In reply to :Gijs (he/him) from comment #6)

Sorry, I've just realized this was here - please could you file a separate github ticket at https://github.com/mozilla-mobile/fenix ? The implementation of alert() is not the same between mobile and desktop and so the fenix team will need to look at this separately. Thank you!

Related issues: https://github.com/mozilla-mobile/fenix/issues/21122

Comment 10

[Greg Stoll :gstoll]

Attachment: Bug 1754759 - remove modal state from tab when moving to new window r=Gijs!

Comment 11

[Paul Zühlcke [:pbz]]

Here is a log for the modal state enter / exit calls. We seem do be doing it in both PromptParent and Prompter. Not necessarily the root cause of the bug but still unexpected.

// Prompt Opens

console.trace: "Prompter.sys.mjs:enterModalState"
resource://gre/modules/Prompter.sys.mjs 1182 openPrompt
resource://gre/modules/Prompter.sys.mjs 1049 openPromptSync
resource://gre/modules/Prompter.sys.mjs 1354 alert
https://bug1754759.bmoattachments.org/attachment.cgi?id=9263291 3 

console.trace: "browser: Entering modal state"
chrome://global/content/elements/browser-custom-element.js 1868 enterModalState
resource:///actors/PromptParent.sys.mjs 163 openPromptWithTabDialogBox
resource:///actors/PromptParent.sys.mjs 103 receiveMessage

// Prompt closes

console.trace: "browser: maybe leave modal state"
chrome://global/content/elements/browser-custom-element.js 1887 maybeLeaveModalState
resource:///actors/PromptParent.sys.mjs 266 openPromptWithTabDialogBox

console.trace: "Prompter.sys.mjs:leaveModalState"
resource://gre/modules/Prompter.sys.mjs 1222 openPrompt
resource://gre/modules/Prompter.sys.mjs 1060 openPromptSync
resource://gre/modules/Prompter.sys.mjs 1354 alert
https://bug1754759.bmoattachments.org/attachment.cgi?id=9263291 3 

JavaScript error: resource://gre/modules/Prompter.sys.mjs, line 1213: NS_ERROR_NOT_AVAILABLE: prompt aborted by user

The "prompt aborted by user" error comes from here:

commonDialogOnLoad (chrome://global/content/commonDialog.js#109)
close (resource://gre/modules/SubDialog.sys.mjs#446)
abort (resource://gre/modules/SubDialog.sys.mjs#232)
abortDialogs (resource://gre/modules/SubDialog.sys.mjs#1095)
abortDialogs (resource://gre/modules/SubDialog.sys.mjs#1095)
abortAllDialogs (chrome://browser/content/browser.js#7607)
handleEvent (chrome://browser/content/browser.js#7602)
_beginRemoveTab (chrome://browser/content/tabbrowser/tabbrowser.js#4314)
swapBrowsersAndCloseOther (chrome://browser/content/tabbrowser/tabbrowser.js#4701)
swapBrowsers (chrome://browser/content/browser-init.js#264)
onLoad (chrome://browser/content/browser-init.js#276)
<anonymous> (chrome://browser/content/browser.xhtml#126)

The tab move triggers a "TabClose" event which then calls into abortAllDialogs and then bubbles up as the error message "prompt aborted by user" via commonDialog.js and Prompter.sys.mjs here: https://searchfox.org/mozilla-central/rev/4582d908c17fbf7924f5699609fe4a12c28ddc4a/toolkit/components/prompts/src/Prompter.sys.mjs#1211

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9d34a121d244
remove modal state from tab when moving to new window r=tabbrowser-reviewers,pbz,mak

Comment 13

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/9d34a121d244

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:gstoll, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox128 to wontfix.

For more information, please visit BugBot documentation.

Comment 15

[Greg Stoll :gstoll]

I don't think this is worth uplifting to beta, but note that I do plan on uplifting to ESR 128.1 since DLP functionality will be there and this also fixes bug 1898733.

Comment 16

[Greg Stoll :gstoll]

Attachment: Bug 1754759 - remove modal state from tab when moving to new window

Original Revision: https://phabricator.services.mozilla.com/D213156

Comment 17

[Phabricator Automation]

esr128 Uplift Approval Request

• User impact if declined: moving a page to a new window with a tab-modal dialog will make it impossible to interact with
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: n/a
• Risk associated with taking this patch: low
• Explanation of risk level: has been in mainline since Fx 129 and has tests
• String changes made/needed: no
• Is Android affected?: no

Comment 18

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/25025c12a9f4