Marionette is not able to detect and handle user prompts on Android
Categories
(Remote Protocol :: Marionette, defect, P3)
Tracking
(firefox100 fixed)
Tracking | Status | |
---|---|---|
firefox100 | --- | fixed |
People
(Reporter: whimboo, Assigned: olivia)
References
()
Details
(Whiteboard: [geckoview:98])
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
While working on bug 1686741 to add support for the new content modal type I also tried how Marionette handles prompts on Android. Sadly we still fail in catching any of those. Means even for a simple alert('foo')
Marionette hangs.
Agi, are there specific observer notifications that we would have to listen for to detect new modal dialogs to be opened? Both the common-dialog-loaded
and tabmodal-dialog-loaded
aren't getting fired.
Comment 1•3 years ago
|
||
Hey Henrik! What are you trying to do here? I don't think Android uses any of the Dialog stuff from toolkit to display prompts, that's why you don't see those events. Do you want to always dismiss prompts? list all currently active prompts? something else?
Our prompt code is here btw (for the most part): https://searchfox.org/mozilla-central/source/mobile/android/components/geckoview/GeckoViewPrompt.jsm
Reporter | ||
Comment 2•3 years ago
|
||
(In reply to Agi Sferro | :agi | ni? for questions | ⏰ PST | he/him from comment #1)
Hey Henrik! What are you trying to do here? I don't think Android uses any of the Dialog stuff from toolkit to display prompts, that's why you don't see those events. Do you want to always dismiss prompts? list all currently active prompts? something else?
What we need is the following:
- Detect when a prompt / modal dialog gets opened. This is needed so that a running command in Marionette can be aborted (as defined in the WebDriver specification)
- Query a given tab/window for an open prompt / dialog
- Get the prompt / dialog text
- Set the prompt / dialog text
- Accept the prompt / dialog
- Dismiss the prompt / dialog
I hope that there will be a way to get all of these features for Marionette. I'm happy to file bugs for individual requests if those don't exist yet. Most important right now would be 1, 2, and 6 (by default a new command dismisses an open prompt).
I had a look at the mentioned JS module but it looks to be all internal code, which isn't that helpful to get our above use cases implemented for Firefox on Android.
Comment 3•3 years ago
|
||
So we don't really keep any prompt state around right now, so most of these will need significant changes.
Detect when a prompt / modal dialog gets opened. This is needed so that a running command in Marionette can be aborted (as defined in the WebDriver specification)
for this we could send an event here: https://searchfox.org/mozilla-central/rev/6371054f6260a5f8844846439297547f7cfeeedd/mobile/android/components/geckoview/GeckoViewPrompter.jsm#118 to let you know that we're opening a prompt, and it would have all the information like text/window/etc
Query a given tab/window for an open prompt / dialog
right now we have no knowledge of what window has a prompt opened
Get the prompt / dialog text
you could do this with the event above
Set the prompt / dialog text
There's not really an API to do that right now, what is the purpose of this? doesn't marionette run in automation?
Accept the prompt / dialog
Dismiss the prompt / dialog
So we could do this using the event above, you could call something like prompter.dismiss() or whatever and Gecko will think the prompt doesn't exist anymore, but the UI won't be notified as right now we don't really have an API to notify of such events (in a normal browser the user is the only one that can dismiss a prompt).
We've discussed adding it for other reasons (e.g. when the page navigates) but I'm not sure if the UI piece of this is what's important to you or the Gecko side.
We can discuss this further in matrix/zoom if you prefer
Reporter | ||
Comment 4•3 years ago
|
||
(In reply to Agi Sferro | :agi | ni? for questions | ⏰ PST | he/him from comment #3)
Detect when a prompt / modal dialog gets opened. This is needed so that a running command in Marionette can be aborted (as defined in the WebDriver specification)
for this we could send an event here: https://searchfox.org/mozilla-central/rev/6371054f6260a5f8844846439297547f7cfeeedd/mobile/android/components/geckoview/GeckoViewPrompter.jsm#118 to let you know that we're opening a prompt, and it would have all the information like text/window/etc
In Gecko-based applications on desktop like Firefox and Thunderbird we register for observers. Does that need to be an event in GeckView? If not what would be the benefit in having an event compared to an observer notification?
Query a given tab/window for an open prompt / dialog
right now we have no knowledge of what window has a prompt opened
This feature has a lower priority given that in nearly all the cases a modal will be opened in the currently selected window. But there could be cases when sites in background tabs open a user prompt on their own.
Get the prompt / dialog text
you could do this with the event above
In Firefox/Thunderbird we currently create a dialog
instance that refers to the stored dialog. Given that the latter is not available we might have to pass-in an optional text, which would need to be set for Firefox on Android until dialogs get stored.
Set the prompt / dialog text
There's not really an API to do that right now, what is the purpose of this? doesn't marionette run in automation?
This is to handle user prompts of type confirm
, which require an input from the user. It's actually not the description as shown, and as such might be confusing.
Accept the prompt / dialog
Dismiss the prompt / dialogSo we could do this using the event above, you could call something like prompter.dismiss() or whatever and Gecko will think the prompt doesn't exist anymore, but the UI won't be notified as right now we don't really have an API to notify of such events (in a normal browser the user is the only one that can dismiss a prompt).
We've discussed adding it for other reasons (e.g. when the page navigates) but I'm not sure if the UI piece of this is what's important to you or the Gecko side.
So accepting / dismissing the event will not be handled right away by the command that is currently running. Instead the behavior can be customized, and the next command handles the prompt. In Firefox/Thunderbird we directly interact with the user prompt ui.
We can discuss this further in matrix/zoom if you prefer
I'm happy to further discuss via Zoom, which may be best to find the required list of features as required by Marionette. Thanks.
Reporter | ||
Comment 5•3 years ago
|
||
Agi, could it be that the same problem exists for usual popups as raised by e.g multi-select options?
Comment 6•3 years ago
|
||
(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #4)
(In reply to Agi Sferro | :agi | ni? for questions | ⏰ PST | he/him from comment #3)
Detect when a prompt / modal dialog gets opened. This is needed so that a running command in Marionette can be aborted (as defined in the WebDriver specification)
for this we could send an event here: https://searchfox.org/mozilla-central/rev/6371054f6260a5f8844846439297547f7cfeeedd/mobile/android/components/geckoview/GeckoViewPrompter.jsm#118 to let you know that we're opening a prompt, and it would have all the information like text/window/etc
In Gecko-based applications on desktop like Firefox and Thunderbird we register for observers. Does that need to be an event in GeckView? If not what would be the benefit in having an event compared to an observer notification?
sorry with event I meant an observer notification :) it would be very similar to what happens on Desktop, except the object class would be different.
Get the prompt / dialog text
you could do this with the event above
In Firefox/Thunderbird we currently create a
dialog
instance that refers to the stored dialog. Given that the latter is not available we might have to pass-in an optional text, which would need to be set for Firefox on Android until dialogs get stored.
I'm not sure what you mean with "the latter is not available". I'll bring it up on the zoom call.
Set the prompt / dialog text
There's not really an API to do that right now, what is the purpose of this? doesn't marionette run in automation?
This is to handle user prompts of type
confirm
, which require an input from the user. It's actually not the description as shown, and as such might be confusing.
Ah that makes a lot of sense! Then yeah we can support this in the same way we would accept/dismiss.
Accept the prompt / dialog
Dismiss the prompt / dialogSo we could do this using the event above, you could call something like prompter.dismiss() or whatever and Gecko will think the prompt doesn't exist anymore, but the UI won't be notified as right now we don't really have an API to notify of such events (in a normal browser the user is the only one that can dismiss a prompt).
We've discussed adding it for other reasons (e.g. when the page navigates) but I'm not sure if the UI piece of this is what's important to you or the Gecko side.So accepting / dismissing the event will not be handled right away by the command that is currently running. Instead the behavior can be customized, and the next command handles the prompt. In Firefox/Thunderbird we directly interact with the user prompt ui.
Yeah that's fine, marionette can call the methods whenever it wants (everything is asynchronous around prompts).
We can discuss this further in matrix/zoom if you prefer
I'm happy to further discuss via Zoom, which may be best to find the required list of features as required by Marionette. Thanks.
I'll set up something on your calendar.
Reporter | ||
Comment 7•3 years ago
|
||
(In reply to Agi Sferro | :agi | ni? for questions | ⏰ PST | he/him from comment #6)
In Gecko-based applications on desktop like Firefox and Thunderbird we register for observers. Does that need to be an event in GeckView? If not what would be the benefit in having an event compared to an observer notification?
sorry with event I meant an observer notification :) it would be very similar to what happens on Desktop, except the object class would be different.
That sound good then! It means lesser work to get the integration in Marionette done.
Get the prompt / dialog text
you could do this with the event above
In Firefox/Thunderbird we currently create a
dialog
instance that refers to the stored dialog. Given that the latter is not available we might have to pass-in an optional text, which would need to be set for Firefox on Android until dialogs get stored.I'm not sure what you mean with "the latter is not available". I'll bring it up on the zoom call.
So in Firefox the dialog can be accessed via window.Dialog
. But to keep cross-application support Marionette has its own wrapper:
https://searchfox.org/mozilla-central/rev/aec7c53cdbbff65305d41c9d805a70efc0e902ed/testing/marionette/modal.js#262
That means that GeckoView might have to store the dialog somewhere so that it's related to the currently selected tab. Switching through tabs should allow us to query for a currently open prompt like:
Maybe that helps a bit to get a better understanding of how prompts are handled.
Reporter | ||
Comment 8•3 years ago
|
||
I discussed this topic last week with Agi in person. Agi mind adding all the bugs that the GeckoView team has to fix as blockers for this bug? Thanks.
Comment 9•3 years ago
|
||
Done! There's also Bug 1710666 which is relevant for marionette/ Web Driver but not a blocker.
Reporter | ||
Comment 10•3 years ago
|
||
Thanks Agi! Will follow the dependencies to know upfront when we can get this feature added to Marionette.
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 11•3 years ago
|
||
Added _prompts to window to collect active prompts. Added to the prompt object a way to get the sent prompt message text, callback, and accept prompt(), confirm(), and alert().
Updated•3 years ago
|
Updated•3 years ago
|
Reporter | ||
Comment 12•3 years ago
|
||
Hi Olivia, I wanted to ask if you were able to make progress or if there are issues you need help with. Please let me know where I can help. Thanks.
Assignee | ||
Comment 13•3 years ago
|
||
Hey Henrik,
Thanks, I'm working through getting the tests in users_prompts.py
still. If you have time next week, would you mind going through that with me on zoom? I think I might need to make some additions to model.js
to bring everything together. Currently, I'm having an exception here, after it creates the dialog.
Reporter | ||
Comment 14•3 years ago
|
||
Sure we can do. But if you already have some updated code that I could apply locally I'm happy to have a look at before me meet. Maybe it's easier to spot. Other option is to actually push to try with my patches applied on your stack.
Assignee | ||
Comment 15•3 years ago
|
||
(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #14)
Sure we can do. But if you already have some updated code that I could apply locally I'm happy to have a look at before me meet. Maybe it's easier to spot. Other option is to actually push to try with my patches applied on your stack.
Thanks, I'll give that a try! I made some changes today that I'm still working through that seem to be in the right direction - I'll push that soon.
Reporter | ||
Comment 16•3 years ago
|
||
I had a quick look on your patches and when running the user_prompts.py tests I noticed that no prompts are actually opened by the appropriate execute script calls which use i.e. window.alert("foo")
. As such I connected via remote debugging and run this command in the console with the same outcome.
So how am I able to open alerts, prompts, and confirm dialogs in GeckoView? Why does the above not work? Or is that related to the test runner application?
Reporter | ||
Comment 17•3 years ago
|
||
(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #16)
So how am I able to open alerts, prompts, and confirm dialogs in GeckoView? Why does the above not work? Or is that related to the test runner application?
So it's actually the GeckoView testrunner application which doesn't seem to support prompts. Using GeckoView Example instead prompts can be opened.
Agi, what effort would it be to have prompt support in the testrunner application? Note that nearly all internal tests make use of that app, so I'm inclined to follow that strategy. If it's not possible or would take a long time maybe we can temporarily use the example app. Thanks!
Comment 18•3 years ago
|
||
I explained this a little bit on Matrix, but the gist is that test_runner does not connect any of the UI bits for prompts, as it's only intended to be run in a test environment. All the Gecko bits for prompts should still be there, so marionette should still be able to see the prompts (even though no actual UI is painted for it). If any of the prompts don't work for whatever reason we can look into that.
Updated•3 years ago
|
Updated•3 years ago
|
Reporter | ||
Comment 19•3 years ago
|
||
I tested the two patches and there is at least one more failure when running the tests under send_alert_text/send.py
:
> assert response.status == 200, str(response.error)
E AssertionError: unsupported operation (500): User prompt of type undefined is not supported
E
E Remote-end stacktrace:
E
E WebDriverError@chrome://remote/content/shared/webdriver/Errors.jsm:183:5
E UnsupportedOperationError@chrome://remote/content/shared/webdriver/Errors.jsm:521:5
E GeckoDriver.prototype.sendKeysToDialog@chrome://remote/content/marionette/driver.js:2514:13
E
E assert 500 == 200
E +500
E -200
The issue is that this.dialog.args.promptType
returns undefined
:
https://searchfox.org/mozilla-central/rev/6050205f3174fd24c2b6be11c69ecd788cd2b6b3/remote/marionette/driver.js#2494
Here I only run the test test_unexpected_alert
but it should apply to others in this file as well. To actually run all the tests you will have to comment out the session
fixture in the conftest.py
file within the same folder.
Olivia, can you please check?
Updated•3 years ago
|
Assignee | ||
Comment 20•3 years ago
|
||
(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #19)
I tested the two patches and there is at least one more failure when running the tests under
send_alert_text/send.py
:> assert response.status == 200, str(response.error) E AssertionError: unsupported operation (500): User prompt of type undefined is not supported E E Remote-end stacktrace: E E WebDriverError@chrome://remote/content/shared/webdriver/Errors.jsm:183:5 E UnsupportedOperationError@chrome://remote/content/shared/webdriver/Errors.jsm:521:5 E GeckoDriver.prototype.sendKeysToDialog@chrome://remote/content/marionette/driver.js:2514:13 E E assert 500 == 200 E +500 E -200
The issue is that
this.dialog.args.promptType
returnsundefined
:
https://searchfox.org/mozilla-central/rev/6050205f3174fd24c2b6be11c69ecd788cd2b6b3/remote/marionette/driver.js#2494Here I only run the test
test_unexpected_alert
but it should apply to others in this file as well. To actually run all the tests you will have to comment out thesession
fixture in theconftest.py
file within the same folder.Olivia, can you please check?
Hi Henrik,
Thanks, I added the promptType
to the args. I believe it is working as expected now, for send_alert_text/send.py
, most tests pass. The two tests that fail are now failing for this reason: unsupported operation (500): openTab() not supported in GeckoView Test Runner
.
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Comment 21•3 years ago
|
||
Pushed by ohall@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ffedefe9258c [marionette] Add support for user prompts on Android. r=webdriver-reviewers,whimboo
Assignee | ||
Updated•3 years ago
|
Comment 23•3 years ago
|
||
Pushed by ohall@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2d6bf4ec9c40 [marionette] Add support for user prompts on Android. r=webdriver-reviewers,whimboo
Comment 24•3 years ago
|
||
bugherder |
Assignee | ||
Updated•3 years ago
|
Updated•2 years ago
|
Description
•