w3c / webdriver-bidi

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

Prompt handler #681

Closed jgraham closed 1 week ago

jgraham commented 3 months ago

Preview | Diff

jgraham commented 3 months ago

https://github.com/whatwg/html/pull/10189 is the HTML side of this change. It also depends on landing https://github.com/w3c/webdriver/pull/1791

whimboo commented 3 weeks ago

@jgraham as it looks like you missed to specify the steps for handling a user prompt.

whimboo commented 3 weeks ago

@jgraham as it looks like you missed to specify the steps for handling a user prompt.

Please disregard this comment. I was searching for beforeunload in the preview document but the paragraph here lists the prompt as prompt to unload steps.

css-meeting-bot commented 3 weeks ago

The Browser Testing and Tools Working Group just discussed Prompt Handler for beforeunload.

The full IRC log of that discussion <jgraham> Topic: Prompt Handler for beforeunload
<jgraham> github: https://github.com/w3c/webdriver-bidi/pull/681
<orkon> jgraham: the next topic is about prompt handling, not only the beforeunload. The context is: there is a PR to add the prompt handling support based on the WebDriver spec changes. It allows to specify defaults for the BiDi only sessions. In WebDriver the handler applied to all prompt types except for beforeunload. In BiDi, you can get an event that
<orkon> there was a beforeunload prompt and the client can handle it. But that means that the way things are specified in Classic, default prompt handler does not apply to beforeunload. The question is if we want to carry this over to by and what to do when we start treating file dialogs and auth prompts. The default in classic to always ignore. And for
<orkon> file dialogs the accept does not make sense as a value. What should we do about that?
<jgraham> https://github.com/w3c/webdriver-bidi/pull/681#discussion_r1634703120
<orkon> jgraham: there are multiple options. For example, we can overwrite the behavior in Classic. We could change the semantics but it might mean that things might break when updating from Classic to BiDi. We could also only allow ignore and dismiss in BiDi or remove the default.
<orkon> q+
<jgraham> ScribeNick: jgraham
<jgraham> ack next
<jgraham> orkon: We discussed on the PR. From our perspective changing the semantics and having the default apply to all prompt types seems best for BiDi. But not sure about compatibility with classic. If you're updating from classic, users might expect the default to apply to all dialogs including beforeUnload, so maybe it would fix expectations.
<jgraham> orkon: In general it's not a blocker for this proposal. We could live with accept not working for all prompt types.
<orkon> ScribeNick: orkon
<orkon> orkon: we could just make it completely breaking if you switch to BiDi. We could make it than we support only what we currently support. You can still make it work for classic+bidi session so that the syntax changes then.
<orkon> s/orkon:/jgraham:
<simonstewart> q+
<orkon> jgraham: question to the selenium users: do you want to do more with the prompt handling compared to classic?
<jgraham> ack simonstewart
<orkon> simonstewart: in the selenium project, we will be migrating the users to WebDriver BiDi as soon as possible
<jimevans> q+
<orkon> simonstewart: everyone knows that the long term future will be BiDi. The only thing we need Classic for, for teh browsers that do not support BiDi
<jgraham> ack next
<orkon> jimevans: also in selenium, we can, for a mixed session, tailor the behavior of the prompt handler for the prompts that are not part of the classic spec to match the existing behavior. Because once we enable BiDi in Selenium, we can have in one go update our methods that do prompt handling to accomodate the behavior so that the user experience is
<orkon> no different
<orkon> jimevans: so I do not see a problem with having the prompt handlers being different for bidi than for classic
<orkon> q?
<orkon> jgraham: ok what I should try is for classic: you can either provide a string value or you can provide an object and if you provide a default key on the object, then it is the default for everything. And specific keys need to be defined to override the default. Then any navigation will fail if you do not re-define the default for the beforeunload.
<orkon> If we cancel the prompt, the navigation is also cancelled. I think it is compatible with classic but also has better semantics for BiDi
<orkon> q?
OrKoN commented 2 weeks ago

@jgraham so to summarize we will move on with allowing "accept" in the default values or do we still want to completely decouple from classic and drop "accept" in the default configuration and for prompts where it does not make sense? I am not sure if the PR is already updated with the latest discussions? I will take another look if it is ready.

jgraham commented 2 weeks ago

My take is that we should allow "accept" in the default configuration, but for file it should act like cancel unless we extend with some future way to provide a default path. We kind of already do that for alert where dismiss and accept do the same thing.

But I'm also more or less OK with dropping accept from the possible default values. However it does mean you basically always have to override beforeUnload unless you actually want that to cancel navigations with unload prompts, at which point I think the default value isn't that useful at all.