Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[WebDriver BiDi] missing unhandledPromptBehavior #46895

Merged
merged 3 commits into from
Jun 26, 2024
Merged

[WebDriver BiDi] missing unhandledPromptBehavior #46895

merged 3 commits into from
Jun 26, 2024

Conversation

sadym-chromium
Copy link
Contributor

@sadym-chromium sadym-chromium commented Jun 25, 2024 •
edited
Loading

The default unhandledPromptBehavior falls back to dismiss, which does not allow testing prompt handling. This PR provides ignore unhandledPromptBehavior to keep the prompts for testing.

  • add missing handler field to browsingContext.userPromptOpened events.

@sadym-chromium
[WebDriver BiDi] missing unhandledPromptBehavior
e96286a 
Provide missing `unhandledPromptBehavior` capabilities to override default `dismiss` behavior and allow for prompt handling tests.
@sadym-chromium
Copy link
Contributor Author

@whimboo WDYT?

@sadym-chromium sadym-chromium enabled auto-merge (squash) June 25, 2024 13:42
@sadym-chromium sadym-chromium mentioned this pull request Jun 25, 2024
Merged
@whimboo
Copy link
Contributor

whimboo commented Jun 25, 2024

@sadym-chromium I think that this makes sense when we want to test prompt handling for the BiDi upgrade path. I would still have to take a closer looks to the changes.

What I can see is that Chrome is now always failing the tests because of cannot parse capability: unhandledPromptBehavior. Why doesn't it happen without the patch?

@sadym-chromium
Copy link
Contributor Author

sadym-chromium commented Jun 26, 2024 •
edited
Loading

What I can see is that Chrome is now always failing the tests because of cannot parse capability: unhandledPromptBehavior. Why doesn't it happen without the patch?

Chromedriver does not yet support dictionary in the unhandledPromptBehavior capability. Before this patch the capability was missing, and falled back to dismiss. With this patch, the capability yet cannot be parsed by chromedriver.

sadym-chromium added a commit to GoogleChromeLabs/chromium-bidi that referenced this pull request Jun 26, 2024
@sadym-chromium @browser-automation-bot
feat!: respect unhandledPromptBehavior capability (#2351)
08672b9 
BREAKING CHANGE: default behavior changed from `ignore` to `dismiss`.

Failing WPT tests should be addressed by
web-platform-tests/wpt#46895.

* Puppeteer failures are expected, as Puppeteer has to be updated after
Mapper release to pass `unhandledPromptBehavior: {default: 'ignore'}`.
* chromedriver has to pass the capability to Mapper.

---------

Signed-off-by: Browser Automation Bot <[email protected]>
Co-authored-by: Browser Automation Bot <[email protected]>
@sadym-chromium sadym-chromium merged commit 1d4b4f0 into master Jun 26, 2024
17 checks passed
@sadym-chromium sadym-chromium deleted the sadym/unahandledPromptBehavior branch June 26, 2024 09:35
@sadym-chromium
Copy link
Contributor Author

@whimboo oops, it was auto-merged. Please still feel free to provide feedback which can be addressed in another PR.

@sadym-chromium sadym-chromium mentioned this pull request Jun 26, 2024
Merged
@whimboo
Copy link
Contributor

whimboo commented Jun 26, 2024

@whimboo oops, it was auto-merged. Please still feel free to provide feedback which can be addressed in another PR.

I'm a bit afraid that all the tests are failing with Firefox now, but maybe it revealed a bug on our side.

@sadym-chromium
Copy link
Contributor Author

I'm a bit afraid that all the tests are failing with Firefox now, but maybe it revealed a bug on our side.

Looks like you don't provide 'handler': 'dismiss' in the browsingContext.userPromptOpened event.

@whimboo
Copy link
Contributor

whimboo commented Jun 26, 2024

Oh, good point. We indeed missed to add this field! I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1904822 to get this fixed soon.

@whimboo
Copy link
Contributor

whimboo commented Jun 27, 2024 •
edited
Loading

Also, one other observation: the automatic user prompt handling is not yet included in any spec. There is an open pull request on the WHATWG HTML repository that has not been reviewed yet. Therefore, these changes were not strictly necessary, but they are future-proof.

Please note that Firefox has not implemented this feature yet. For tracking purposes, I have filed a bug report: https://bugzilla.mozilla.org/show_bug.cgi?id=1905086.

@whimboo
Copy link
Contributor

whimboo commented Jun 27, 2024

@sadym-chromium as it looks like that Chromium already supports that would you mind writing wdspec tests for this feature?

sadym-chromium added a commit that referenced this pull request Jul 18, 2024
@sadym-chromium
[WebDriver BiDi] missing unhandledPromptBehavior (#46895)
6cfe2bf 
Provide missing `unhandledPromptBehavior` capabilities to override default `dismiss` behavior and allow for prompt handling tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@whimboo whimboo whimboo left review comments

@OrKoN OrKoN OrKoN approved these changes

@AutomatedTester AutomatedTester Awaiting requested review from AutomatedTester

@jgraham jgraham Awaiting requested review from jgraham

@jrandolf-2 jrandolf-2 Awaiting requested review from jrandolf-2

@juliandescottes juliandescottes Awaiting requested review from juliandescottes

@shs96c shs96c Awaiting requested review from shs96c

@Lightning00Blade Lightning00Blade Awaiting requested review from Lightning00Blade

Assignees

@whimboo whimboo

Labels
webdriver-bidi
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sadym-chromium @whimboo @OrKoN @wpt-pr-bot