Closed Bug 1708105 Opened 3 years ago Closed 2 years ago

Marionette is not able to detect and handle user prompts on Android

Categories

(Remote Protocol :: Marionette, defect, P3)

Default
defect

Tracking

(firefox100 fixed)

RESOLVED FIXED
100 Branch
Tracking Status
firefox100 --- fixed

People

(Reporter: whimboo, Assigned: olivia)

References

()

Details

(Whiteboard: [geckoview:98])

Attachments

(1 file)

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.

Flags: needinfo?(agi)

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

Flags: needinfo?(agi) → needinfo?(hskupin)

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

  1. 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)
  2. Query a given tab/window for an open prompt / dialog
  3. Get the prompt / dialog text
  4. Set the prompt / dialog text
  5. Accept the prompt / dialog
  6. 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.

Flags: needinfo?(hskupin) → needinfo?(agi)
Summary: Marionette fails to detect opened user prompts on Android → Marionette is not able to detect and handle user prompts on Android
Blocks: 1699044

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

Flags: needinfo?(agi)

(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 / 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.

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.

Flags: needinfo?(agi)

Agi, could it be that the same problem exists for usual popups as raised by e.g multi-select options?

(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 / 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.

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.

Flags: needinfo?(agi)

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

https://searchfox.org/mozilla-central/rev/aec7c53cdbbff65305d41c9d805a70efc0e902ed/testing/marionette/modal.js#39-94

Maybe that helps a bit to get a better understanding of how prompts are handled.

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.

Flags: needinfo?(agi)

Done! There's also Bug 1710666 which is relevant for marionette/ Web Driver but not a blocker.

Depends on: 1710668
Flags: needinfo?(agi)
Depends on: 1712414

Thanks Agi! Will follow the dependencies to know upfront when we can get this feature added to Marionette.

No longer blocks: 1699044
Blocks: 1729409
Assignee: nobody → ohall

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

Priority: P3 → P1
Whiteboard: [geckoview:98]
Priority: P1 → P3
Blocks: 1559120
No longer blocks: 1729409

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.

Status: NEW → ASSIGNED
Flags: needinfo?(ohall)

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.

Flags: needinfo?(ohall) → needinfo?(hskupin)

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.

Flags: needinfo?(hskupin) → needinfo?(ohall)

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

Blocks: 1749444

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?

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

Flags: needinfo?(agi)

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.

Flags: needinfo?(agi)
Attachment #9256452 - Attachment description: WIP: Bug 1708105 - GeckoView user prompt options for Marionette → WIP: Bug 1708105 - GeckoView user prompt options for Marionette Added option to find GeckoView user prompts from the geckoview.js window.
Attachment #9256452 - Attachment description: WIP: Bug 1708105 - GeckoView user prompt options for Marionette Added option to find GeckoView user prompts from the geckoview.js window. → WIP: Bug 1708105 - GeckoView user prompt options for Marionette

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?

Attachment #9256452 - Attachment description: WIP: Bug 1708105 - GeckoView user prompt options for Marionette → Bug 1708105 - GeckoView user prompt options for Marionette

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

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.

Flags: needinfo?(ohall)
Attachment #9256452 - Attachment description: Bug 1708105 - GeckoView user prompt options for Marionette → Bug 1708105 - [marionette] Add support for user prompts on Android.
Attachment #9256452 - Attachment description: Bug 1708105 - [marionette] Add support for user prompts on Android. → Bug 1708105 - [marionette] Add support for user prompts on Android
Attachment #9256452 - Attachment description: Bug 1708105 - [marionette] Add support for user prompts on Android → Bug 1708105 - [marionette] Add support for user prompts on Android.
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
Flags: needinfo?(ohall)
See Also: → 1761480
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
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 100 Branch
Flags: needinfo?(ohall)
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: