Open Bug 1686741 Opened 1 month ago Updated 10 days ago

Add complete support for the new tab modal dialog

Categories

(Testing :: Marionette, enhancement, P3)

Firefox 86
enhancement

Tracking

(firefox-esr78 unaffected, firefox84 unaffected, firefox85 unaffected, firefox86 affected)

ASSIGNED
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.

No longer regressed by: 1680637
Regressed by: 1680637

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

Type: defect → enhancement
Priority: -- → P3
Summary: Test new modal dialog for Marionette → Add complete support for the new modal dialog
Version: Default → Firefox 86

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?

Flags: needinfo?(mtigley)

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 call TabDialogBox.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.

Flags: needinfo?(mtigley)

(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?

Flags: needinfo?(mtigley)
Assignee: nobody → hskupin
Status: NEW → ASSIGNED

(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 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?

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).

Flags: needinfo?(mtigley)

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
Summary: Add complete support for the new modal dialog → Add complete support for the new tab modal dialog
Depends on: 1560015

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:

https://searchfox.org/mozilla-central/rev/b32d4ca055ca9cf717be480df640f8970724a0ce/testing/marionette/harness/marionette_harness/www/test_tab_modal_dialogs.html#27-30,37

Is there any kind of additional logging available for prompts? Not sure how to best investigate that race condition.

Flags: needinfo?(mtigley)

We clarified the problem, and actually have to wait for the second user prompt to appear.

Flags: needinfo?(mtigley)

Using the "waitForEvent" promise will cause an extra
"DOMModalDialogClosed" event to be logged, which is
confusing.

You need to log in before you can comment on or make changes to this bug.