"WebDriver:AcceptAlert" and "WebDriver:DismissAlert" can hang for alerts in opened popup windows
Categories
(Remote Protocol :: Marionette, defect, P3)
Tracking
(firefox-esr78 unaffected, firefox90 wontfix, firefox91 fixed, firefox92 fixed)
Tracking | Status | |
---|---|---|
firefox-esr78 | --- | unaffected |
firefox90 | --- | wontfix |
firefox91 | --- | fixed |
firefox92 | --- | fixed |
People
(Reporter: kovalev007, Assigned: whimboo)
References
(Regression)
Details
(Keywords: regression)
Attachments
(6 files)
3.77 KB,
text/plain
|
Details | |
107.97 KB,
text/plain
|
Details | |
129.17 KB,
text/plain
|
Details | |
1.24 KB,
text/plain
|
Details | |
48 bytes,
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
|
Details | Review |
48 bytes,
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
|
Details | Review |
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/92.0.4515.107 Safari/537.36
Steps to reproduce:
Testcase:
https://output.jsbin.com/zelecaj
click on button with text "open"
wait new window opened
click on button with text "alert"
accept alert
Actual results:
geckodriver hung after accept alert
Expected results:
test should be continue
Reporter | ||
Comment 1•3 years ago
|
||
Bug exists in Firefox beta and Firefox nightly
Reporter | ||
Comment 2•3 years ago
|
||
Reporter | ||
Comment 3•3 years ago
|
||
Assignee | ||
Updated•3 years ago
|
Reporter | ||
Comment 4•3 years ago
|
||
From mozregression_log_from_89_to_90.txt
Last good revision: ce8036ee1a2c572f653b79fe42108815f23af15d
First bad revision: 150c94fd5ad36145cb171c217702e9cb7c589793
Pushlog: https://hg.mozilla.org/releases/mozilla-release/pushloghtml?fromchange=ce8036ee1a2c572f653b79fe42108815f23af15d&tochange=150c94fd5ad36145cb171c217702e9cb7c589793
I tried to narrow down my search - with no success
mozregression --good ce8036ee1a2c572f653b79fe42108815f23af15d --bad 150c94fd5ad36145cb171c217702e9cb7c589793 --command 'performance-crawler.bat'
0:02.15 INFO: bits option not specified, using 64-bit builds.
0:03.43 INFO: 150c94fd5ad36145cb171c217702e9cb7c589793 is not a release, assuming it's a hash...
0:04.74 INFO: ce8036ee1a2c572f653b79fe42108815f23af15d is not a release, assuming it's a hash...
0:04.75 INFO: Getting mozilla-central builds between ce8036ee1a2c572f653b79fe42108815f23af15d and 150c94fd5ad36145cb171c217702e9cb7c589793
0:05.95 ERROR: The url 'https://hg.mozilla.org/mozilla-central/json-pushes?changeset=ce8036ee1a2c572f653b79fe42108815f23af15d' returned a 404 error because the push is not in this repo (e.g., not merged yet).
Also I tried use mozregression by days
From mozregression_log_from_2021-04-25_to_202104-26.txt
Last good revision: 82d59bab21dadcad3c63aa1537fc48fa3c8a4e98
First bad revision: 15253a3168d2f2543a5348d47157f033995e928b
Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=82d59bab21dadcad3c63aa1537fc48fa3c8a4e98&tochange=15253a3168d2f2543a5348d47157f033995e928b
Henrik Skupin you can confirm that the regression appeared after Bug 1701686?
Reporter | ||
Comment 5•3 years ago
|
||
Assignee | ||
Comment 6•3 years ago
|
||
Thanks a lot for the report George! It's great to see that you get back to us with such detailed information for regressions. I appreciate that!
I had a look at the example code and I'm able to reproduce it with Marionette itself. It fails since bug 1701686 has been landed. Specifically the following changeset is responsible:
https://hg.mozilla.org/integration/autoland/rev/e352f905ecf8
I assume that the window check if (action == modal.ACTION_CLOSED && window == win) {
is failing here. I'll have a look at it.
We should be able to easily get it fixed for the 91 release.
Assignee | ||
Comment 7•3 years ago
|
||
The problem here is actually that we no longer wait for a DOMModalDialogClosed
event, but rely on the toplevel-window-ready
observer notification, which centrally registers such an event listener for the newly opened window. But that's not done here. Due to whatever reason no toplevel-window-ready
notification is fired when a new window (popup) gets opened. As such no such listener is active.
From the MDN docs I see Called just after a new top level window has been opened and is ready, but has not yet loaded a document
:
https://github.com/mdn/archived-content/blob/main/files/en-us/mozilla/tech/xpcom/observer_notifications/index.html
Olli, do you know why we do not get such an observer? Is there some other notification we should register for? I tried some others from the above page, but none seem to be helpful.
Assignee | ||
Comment 8•3 years ago
|
||
The reason why a new window gets opened here is because status: no
is passed as attribute into the window.open()
call. If that is removed we always open a new tab instead.
Attached can be found a minimized Marionette test to reproduce it. It can be run via:
./mach marionette-test --gecko-log - %path%
Assignee | ||
Comment 9•3 years ago
|
||
As discussed with Arai on Elements it looks like that we will have to switch to the domwindowopened
observer notification. That gets really fired for each and every new window.
Will have to check if that causes some kind of side-effect in marionette through..
Assignee | ||
Comment 10•3 years ago
|
||
FYI there is a standards discussion regarding the topic when a new popup should be opened:
https://github.com/whatwg/html/issues/5872
Given that it should always happen across all browsers when a width and height is specified I will add a new Wdspec test to cover this specific scenario.
Not sure how many pages are out there that use real popups and are affected, but I think it's worth getting fixed for the 91 release. Will take this bug.
Assignee | ||
Comment 11•3 years ago
|
||
Assignee | ||
Comment 12•3 years ago
|
||
Depends on D120733
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Comment 13•3 years ago
|
||
Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2520da75e848 [wdspec] Add test to accept or dismiss a user prompt in a popup window. r=webdriver-reviewers,jdescottes https://hg.mozilla.org/integration/autoland/rev/926af858a573 [marionette] Use domwindowopened observer to detect newly opened windows. r=webdriver-reviewers,jdescottes
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/29782 for changes under testing/web-platform/tests
Comment 15•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2520da75e848
https://hg.mozilla.org/mozilla-central/rev/926af858a573
Assignee | ||
Comment 16•3 years ago
|
||
(In reply to Web Platform Test Sync Bot (Matrix: #interop:mozilla.org) from comment #14)
Created web-platform-tests PR
https://github.com/web-platform-tests/wpt/pull/29782 for changes under
testing/web-platform/tests
James, can you please check why the PR doesn't get automatically merged? Thanks!
Assignee | ||
Comment 17•3 years ago
|
||
Comment on attachment 9232869 [details]
Bug 1721982 - [marionette] Use domwindowopened observer to detect newly opened windows.
Beta/Release Uplift Approval Request
- User impact if declined: If web pages under test by WebDriver open a new popup window any user prompt that gets opened from within this window cannot be closed and Marionette hangs.
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: No
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): The change only affects the handling of user prompts (content, tab, modal) and simply swaps the observer notification to listen for.
- String changes made/needed: no
Assignee | ||
Updated•3 years ago
|
Comment 18•3 years ago
|
||
Comment on attachment 9232869 [details]
Bug 1721982 - [marionette] Use domwindowopened observer to detect newly opened windows.
Approved for 91 beta 8, thanks.
Updated•3 years ago
|
Comment 19•3 years ago
|
||
bugherder uplift |
Upstream PR merged by jgraham
Updated•3 years ago
|
Updated•1 year ago
|
Description
•