search

w3c / webdriver-bidi

Bidirectional WebDriver protocol for browser automation
https://w3c.github.io/webdriver-bidi/
351 stars 38 forks source link

Extend `browsingContext.close` params with `promptUnload` #518

Closed Lightning00Blade closed 10 months ago

Lightning00Blade commented 1 year ago

Closes #462

We already define this behavior on browser.close command. This PR extends this to the browsingContext.close as a parameter. Default it chosen to not trigger prompting to unload with the idea that this is used more like resource cleanup rather then testing beforeUnload prompts and alike.

Preview | Diff

OrKoN commented 11 months ago

@juliandescottes @jgraham @whimboo would it be possible for you to review this one? it seems uncontroversial

OrKoN commented 10 months ago

@jgraham should we land this change or do you have concerns about exposing this?

thiagowfx commented 10 months ago

Is there a bug or PR for the corresponding WPT test?

jgraham commented 10 months ago

I think this is OK in principle.

I also think we should fix this to not work by "magic" but actually modify https://html.spec.whatwg.org/#steps-to-fire-beforeunload to skip the prompt in some cases. It seems like we could reuse the existing WebDriver BiDi user prompt opened steps by allowing them to return a value that specifies whether to show the prompt or not, and keeping a set of browsing contexts for which the unload prompt should be skipped.

That doesn't block landing this one, but we should at least file an issue.

Lightning00Blade commented 10 months ago

Issue on HTML spec - https://github.com/whatwg/html/issues/9890 , please add anything I may have missed. Merging this per the last comment.

whimboo commented 10 months ago

@Lightning00Blade Thanks for your work! Are you going to also create a test for that?

Lightning00Blade commented 10 months ago

Yes I will add WPT test for it, I already updated Chromium-BiDi https://github.com/GoogleChromeLabs/chromium-bidi/pull/1508 to support it, so it should be easier to create them now.

whimboo commented 7 months ago

Yes I will add WPT test for it, I already updated Chromium-BiDi GoogleChromeLabs/chromium-bidi#1508 to support it, so it should be easier to create them now.

Hi @Lightning00Blade. Quick question if you still have it in your queue. Thanks.

whimboo commented 7 months ago

FYI the tests will be added via https://github.com/web-platform-tests/wpt/pull/44004.