Add complete support for the new tab modal dialog
Categories
(Testing :: Marionette, enhancement, P3)
Tracking
(firefox-esr78 unaffected, firefox84 unaffected, firefox85 unaffected, firefox86 affected)
Tracking | Status | |
---|---|---|
firefox-esr78 | --- | unaffected |
firefox84 | --- | unaffected |
firefox85 | --- | unaffected |
firefox86 | --- | affected |
People
(Reporter: mtigley, Assigned: whimboo)
References
(Regression)
Details
Attachments
(2 files)
Marionette tests currently have the new modal UI disabled. This issue is for updating the existing tests.
Assignee | ||
Comment 1•1 month ago
|
||
It's not only around updating the tests in here:
https://searchfox.org/mozilla-central/source/testing/marionette/harness/marionette_harness/tests/unit/test_modal_dialogs.py
Most of the tests actually work, but the ones that create a modal dialog before the Marionette session gets actually started cannot be found.
When starting to work on this bug the preference as set in recommended preferences has to be removed:
https://searchfox.org/mozilla-central/source/testing/marionette/components/marionette.js
Set release status flags based on info from the regressing bug 1680637
Assignee | ||
Comment 3•25 days ago
|
||
Micah, would you have the time to get the support added to Marionette? I would kinda appreciate. If not mind giving some advice in how to use the new modal dialog type?
Reporter | ||
Comment 4•25 days ago
|
||
Hey Henrik, sorry I dropped the ball on this. I've been swept up with other work.
(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #3)
Micah, would you have the time to get the support added to Marionette? I would kinda appreciate. If not mind giving some advice in how to use the new modal dialog type?
Of course! Here are some things we've been using in mochitests to test the new modal dialog type:
-
We can get a modal container for a tab using gBrowser.getTabDialogBox.
-
Modals opened by JS are stored in a separate manager on
TabDialogBox
. To get access to this manager we need to callTabDialogBox.getContentManager
. -
We get access to its opened modal dialogs by accessing its ._dialogs member.
-
To examine the contents of a single dialog. We have to access its
dialog._frame.contentWindow
object.
It might be easier to show an example of all these pieces being used in an existing test. I recently wrote a test for checking the title text of a dialog at: https://searchfox.org/mozilla-central/rev/fd853f4aea89186efdb368e759a71b7a90c2b89c/browser/base/content/test/tabdialogs/browser_tabdialogbox_content_prompts.js#50-53.
Assignee | ||
Comment 5•15 days ago
|
||
(In reply to Micah Tigley [:mtigley] from comment #4)
It might be easier to show an example of all these pieces being used in an existing test. I recently wrote a test for checking the title text of a dialog at: https://searchfox.org/mozilla-central/rev/fd853f4aea89186efdb368e759a71b7a90c2b89c/browser/base/content/test/tabdialogs/browser_tabdialogbox_content_prompts.js#50-53.
Thanks! I seem to have gotten it working for tab modals like alert
and such, but I still wonder about those like for HTTP authentication. Those are still using getTabDialogManager()
, right? So it's only alert
, confirm
, and prompt
that implement the SubDialog path? It means we have to keep those existing checks?
Assignee | ||
Comment 6•15 days ago
|
||
Updated•15 days ago
|
Reporter | ||
Comment 7•15 days ago
|
||
(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #5)
Thanks! I seem to have gotten it working for tab modals like
alert
and such, but I still wonder about those like for HTTP authentication. Those are still usinggetTabDialogManager()
, right? So it's onlyalert
,confirm
, andprompt
that implement the SubDialog path? It means we have to keep those existing checks?
Yes the HTTP auth modal is still using getTabDialogManager
. Both the HTTP auth and alert/confirm/prompt modals used SubDialog, which begins around here: https://searchfox.org/mozilla-central/source/browser/actors/PromptParent.jsm#293-305.
The only difference between them is that prompts like HTTP auth are caused by webcontent but need to look like it's part of secure browser UI whereas alert/confirm/prompt don't need to be. It's mostly a styling difference. And to manage the two prompt types, we store them in separate managers, which are retrievable via getTabDialogManager()
(for "tab" prompt types) or getContentDialogManager()
(for "content" prompt types).
Assignee | ||
Comment 8•11 days ago
|
||
Making this change actually triggers a known intermittent test failure for TestTabModalAlerts.test_handle_two_dialogs
(bug 1559992) pretty permanently. And as it looks like the first "WebDriver:AcceptAlert
command returns to early and as such fires off immediately the next command, whereby the common-dialog-loaded
observer notification is arrived afterward:
Here how it looks like when it works:
1613406564562 Marionette DEBUG 10 -> [0,16,"WebDriver:ElementClick",{"id":"7991fec3-763f-cd44-8dba-302a6b1cc3a5"}]
1613406564605 Marionette TRACE Received observer notification common-dialog-loaded
1613406564605 Marionette TRACE Received observer notification common-dialog-loaded
1613406564605 Marionette TRACE Canceled page load listener because a dialog opened
1613406564605 Marionette DEBUG 10 <- [1,16,null,{"value":null}]
1613406564643 Marionette DEBUG 10 -> [0,17,"WebDriver:SendAlertText",{"text":"foo"}]
1613406564643 Marionette DEBUG 10 <- [1,17,null,{"value":null}]
1613406564646 Marionette DEBUG 10 -> [0,18,"WebDriver:AcceptAlert",{}]
1613406564650 Marionette TRACE Received event DOMModalDialogClosed
1613406564650 Marionette TRACE Received event DOMModalDialogClosed
1613406564650 Marionette TRACE Received DOM event DOMModalDialogClosed for [object XULFrameElement]
1613406564655 Marionette DEBUG 10 <- [1,18,null,{"value":null}]
1613406564712 Marionette TRACE Received observer notification common-dialog-loaded
1613406564712 Marionette TRACE Received observer notification common-dialog-loaded
1613406564712 Marionette DEBUG 10 -> [0,19,"WebDriver:SendAlertText",{"text":"bar"}]
1613406564712 Marionette DEBUG 10 <- [1,19,null,{"value":null}]
And here when it fails:
1613406565071 Marionette DEBUG 11 -> [0,16,"WebDriver:ElementClick",{"id":"65fb781e-b867-f94b-ae23-2f5ccd4c6516"}]
1613406565142 Marionette TRACE Received observer notification common-dialog-loaded
1613406565142 Marionette TRACE Received observer notification common-dialog-loaded
1613406565142 Marionette TRACE Canceled page load listener because a dialog opened
1613406565142 Marionette DEBUG 11 <- [1,16,null,{"value":null}]
1613406565182 Marionette DEBUG 11 -> [0,17,"WebDriver:SendAlertText",{"text":"foo"}]
1613406565182 Marionette DEBUG 11 <- [1,17,null,{"value":null}]
1613406565184 Marionette DEBUG 11 -> [0,18,"WebDriver:AcceptAlert",{}]
1613406565187 Marionette TRACE Received event DOMModalDialogClosed
1613406565187 Marionette TRACE Received event DOMModalDialogClosed
1613406565187 Marionette TRACE Received DOM event DOMModalDialogClosed for [object XULFrameElement]
1613406565188 Marionette DEBUG 11 <- [1,18,null,{"value":null}]
1613406565216 Marionette DEBUG 11 -> [0,19,"WebDriver:SendAlertText",{"text":"bar"}]
1613406565216 Marionette DEBUG 11 <- [1,19,{"error":"no such alert","message":"","stacktrace":"WebDriverError@chrome://marionette/content/error.js:181:5\nNoSuchAl ... t@chrome://marionette/content/server.js:239:9\n_onJSONObjectReady/<@chrome://marionette/content/transport.js:504:20\n"},null]
1613406565222 Marionette TRACE Received observer notification common-dialog-loaded
1613406565222 Marionette TRACE Received observer notification common-dialog-loaded
Assignee | ||
Updated•11 days ago
|
Assignee | ||
Comment 9•10 days ago
|
||
Micah, could it be that the creation of the new tab modals is a bit slower than before? In the above case it's actually for the 2nd prompt as opened from here:
Is there any kind of additional logging available for prompts? Not sure how to best investigate that race condition.
Assignee | ||
Comment 10•10 days ago
|
||
We clarified the problem, and actually have to wait for the second user prompt to appear.
Assignee | ||
Comment 11•10 days ago
|
||
Using the "waitForEvent" promise will cause an extra
"DOMModalDialogClosed" event to be logged, which is
confusing.
Description
•