Provide event to marionette whenever a new prompt is opened
Categories
(GeckoView :: General, enhancement, P1)
Tracking
(firefox100 fixed)
Tracking | Status | |
---|---|---|
firefox100 | --- | fixed |
People
(Reporter: agi, Assigned: olivia)
References
Details
(Whiteboard: [geckoview:m97][geckoview:m98][geckoview:m99][geckoview:m100])
Attachments
(1 file, 1 obsolete file)
(deleted),
text/x-phabricator-request
|
Details |
Marionette needs to know whenever a new prompt is opened, we should fire an observer event every time a prompt is opened with the prompt object so that marionette/Web Driver can integrate with it. See also blocking bug.
Updated•3 years ago
|
Updated•3 years ago
|
Comment 1•3 years ago
|
||
Now that bug 1710668 is fixed, could this feature be implemented?
Comment 2•3 years ago
|
||
Hi, do we have an update for this bug? It would be great to see such support. Thanks.
Assignee | ||
Comment 3•3 years ago
|
||
Updated•3 years ago
|
Assignee | ||
Comment 4•3 years ago
|
||
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Assignee | ||
Comment 5•3 years ago
|
||
Hi Henrik, I added a topic “geckoview-prompt-show” right after we dispatch the event that will open the prompt and it'll have the prompt object. Do you think that'll work for your needs? I’m also starting adding the additional support for 1708105
Comment 6•3 years ago
|
||
Thanks Olivia. I actually already replied in the Phabricator review. Basically it looks fine but I would like to see if Marionette can correctly handle that. I'm happy to mentor you over on bug 1708105. Feel free to join us on Element in the #webdriver channel.
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Comment 7•3 years ago
|
||
Hi Olivia, with your work on prompt support for Marionette (bug 1708105), do you think that the patch on this bug is complete or does it require further changes? If it's all what we need maybe we can indeed get it landed and do all the rest over on the other bug?
Comment 8•3 years ago
|
||
Actually as it looks like most of the additions in D134448 are actually about GeckoView and probably have to be moved to the revision on this bug?
Assignee | ||
Comment 9•3 years ago
|
||
Sure, I can definitely move the more Marionette parts over here! For example, the changes to modal.js
.
I think driver.js
is also going to need a few updates too. What do you think?
For example, at GeckoDriver.prototype.dismissDialog
dismisses the dialogue by clicking the UI button programmatically. I think for GeckoView it will need to call this dismiss()
on GeckoViewPrompter
. I'm working on trying to get the messaging setup so that Marionette can get the GeckoViewPrompter
prompts from the window and then call those methods. Does that sound like the right thing to do?
Assignee | ||
Updated•3 years ago
|
Comment 10•3 years ago
|
||
I've applied your patch to my local tree and tested a bit the two different modes (handling prompts that open during a command is processed, and finding open prompts). Here is what I can see:
-
The modal dialog observer as registered for
geckoview-prompt-show
doesn't actually receive the notification when a prompt gets opened. There should be aReceived observer notification geckoview-prompt-show
in the adb log. As such the just opened prompt is not recognized and a follow up command to accept/dismiss the prompt will fail because there is basically no prompt set. -
If GeckoView cannot provide a UI path to close the dialog we might have to fallback to directly call the appropriate methods. That wouldn't be ideal given that WebDriver is build to actually simulate user interaction but if the platform cannot offer it for us, we should work with the features that we get.
I hope that we can sort out 1) fast enough. 2) doesn't sound to be complex. Let me know if you have further questions.
Comment 11•3 years ago
|
||
(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #10)
- The modal dialog observer as registered for
geckoview-prompt-show
doesn't actually receive the notification when a prompt gets opened. There should be aReceived observer notification geckoview-prompt-show
in the adb log. As such the just opened prompt is not recognized and a follow up command to accept/dismiss the prompt will fail because there is basically no prompt set.
Could this maybe happen because the observer is fired on the child process and not from the parent process?
Assignee | ||
Comment 12•3 years ago
|
||
Thanks for the help! The issue with the observer was 100% that it was firing off of the child process, I moved the observer notification to the parent and it seems to be notifying as expected now.
I'll get back to you on the second issue soon after making some changes, now that the first is working.
Comment 13•3 years ago
|
||
That's great to hear. I'm looking forward in how it will look like when having all the bits together.
Please adjust the two commits so they are attached to the correct bugs. Right now the patch on this bug contains Marionette changes while it's for GeckoView. Thanks!
Updated•3 years ago
|
Assignee | ||
Comment 14•3 years ago
|
||
(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #13)
That's great to hear. I'm looking forward in how it will look like when having all the bits together.
Please adjust the two commits so they are attached to the correct bugs. Right now the patch on this bug contains Marionette changes while it's for GeckoView. Thanks!
Thanks, patches should be switched now!
Updated•3 years ago
|
Comment 15•3 years ago
|
||
It's great to see that we now can run the user prompt tests and don't end-up in timeouts anymore because the events for opening and closing user prompts exist. As Olivia stated several tests are still failing. So I did a quick check and can see that dialogs aren't always closed. As such here some steps that might help to identify the underlying issue:
-
Concentrate on the test
testing/web-platform/tests/webdriver/tests/get_alert_text/get.py
for now and only run the last two tests in that file (rename the others from i.e.test_
totst_
) because here we do not have unsupported features for GeckoView. -
Here the first test opens a prompt with the text
Enter Your Name
. That text is correctly detected but then in test teardown the following error is visible when trying to close the dialog:
03-10 09:46:36.563 8072 8098 I Gecko : 1646901996563 Marionette DEBUG 0 -> [0,11,"WebDriver:DismissAlert",{}]
03-10 09:46:36.564 8072 8098 I Gecko : 1646901996564 Marionette DEBUG 0 <- [1,11,{"error":"no such alert","message":"","stacktrace":"WebDriverError@chrome://remote/content/shared/webdriver/Errors.jsm:183:5\nNoSuchAlertError@chrome://remote/content/shared/webdriver/Errors.jsm:384:5\nGeckoDriver.prototype._checkIfAlertIsPresent@chrome://remote/content/marionette/driver.js:2530:11\nGeckoDriver.prototype.dismissDialog@chrome://remote/content/marionette/driver.js:2419:8\ndespatch@chrome://remote/content/marionette/server.js:306:40\nexecute@chrome://remote/content/marionette/server.js:279:16\nonPacket/<@chrome://remote/content/marionette/server.js:252:20\nonPacket@chrome://remote/content/marionette/server.js:253:9\n_onJSONObjectReady/<@chrome://remote/content/marionette/transport.js:500:20\n"},null]
- This seems to happen because the window is switched before and this command triggers searching for open dialogs in the current window. And as it looks like the new code in Marionette is not able to find the open prompt which results in the prompt being left open. To reproduce that just run
test_get_prompt_text
and add the following lines to the end of the test:
session.window_handle = session.window_handle
session.alert.dismiss()
with pytest.raises(NoSuchAlertException):
session.alert.text
With that fixed a couple of other tests will then pass as well.
Assignee | ||
Comment 16•3 years ago
|
||
(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #15)
It's great to see that we now can run the user prompt tests and don't end-up in timeouts anymore because the events for opening and closing user prompts exist. As Olivia stated several tests are still failing. So I did a quick check and can see that dialogs aren't always closed. As such here some steps that might help to identify the underlying issue:
Concentrate on the test
testing/web-platform/tests/webdriver/tests/get_alert_text/get.py
for now and only run the last two tests in that file (rename the others from i.e.test_
totst_
) because here we do not have unsupported features for GeckoView.Here the first test opens a prompt with the text
Enter Your Name
. That text is correctly detected but then in test teardown the following error is visible when trying to close the dialog:03-10 09:46:36.563 8072 8098 I Gecko : 1646901996563 Marionette DEBUG 0 -> [0,11,"WebDriver:DismissAlert",{}] 03-10 09:46:36.564 8072 8098 I Gecko : 1646901996564 Marionette DEBUG 0 <- [1,11,{"error":"no such alert","message":"","stacktrace":"WebDriverError@chrome://remote/content/shared/webdriver/Errors.jsm:183:5\nNoSuchAlertError@chrome://remote/content/shared/webdriver/Errors.jsm:384:5\nGeckoDriver.prototype._checkIfAlertIsPresent@chrome://remote/content/marionette/driver.js:2530:11\nGeckoDriver.prototype.dismissDialog@chrome://remote/content/marionette/driver.js:2419:8\ndespatch@chrome://remote/content/marionette/server.js:306:40\nexecute@chrome://remote/content/marionette/server.js:279:16\nonPacket/<@chrome://remote/content/marionette/server.js:252:20\nonPacket@chrome://remote/content/marionette/server.js:253:9\n_onJSONObjectReady/<@chrome://remote/content/marionette/transport.js:500:20\n"},null]
- This seems to happen because the window is switched before and this command triggers searching for open dialogs in the current window. And as it looks like the new code in Marionette is not able to find the open prompt which results in the prompt being left open. To reproduce that just run
test_get_prompt_text
and add the following lines to the end of the test:session.window_handle = session.window_handle session.alert.dismiss() with pytest.raises(NoSuchAlertException): session.alert.text
With that fixed a couple of other tests will then pass as well.
Thanks for the tips and help! The user_prompts.py
tests are passing locally for me now.
Updated•3 years ago
|
Comment 17•3 years ago
|
||
Comment 18•3 years ago
|
||
Assignee | ||
Updated•3 years ago
|
Comment 19•3 years ago
|
||
Comment 20•3 years ago
|
||
bugherder |
Description
•