Prompt handler #681
Prompt handler #681
Conversation
|
whatwg/html#10189 is the HTML side of this change. It also depends on landing w3c/webdriver#1791 |
There was a problem hiding this comment.
Sorry for the delay @jgraham. But I decided to wait a bit with further feedback until the general handling of the capability has finished. I've a couple of questions that I've added inline. Maybe you can have a look at these? Thanks.
|
@jgraham as it looks like you missed to specify the steps for handling a user prompt. |
Please disregard this comment. I was searching for |
|
The Browser Testing and Tools Working Group just discussed The full IRC log of that discussion<jgraham> Topic: Prompt Handler for beforeunload<jgraham> github: https://github.com//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//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? |
There was a problem hiding this comment.
I think it looks fine now. If I detect something not working as expected we can do it in a follow-up PR.
Tests will be added as part of my work on https://bugzilla.mozilla.org/show_bug.cgi?id=1824220
|
@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. |
|
My take is that we should allow "accept" in the default configuration, but for But I'm also more or less OK with dropping |
index.bs
Outdated
| @@ -4514,6 +4541,7 @@ failed</dfn> steps given |context| and |navigation status|: | |||
| browsingContext.UserPromptClosedParameters = { | |||
| context: browsingContext.BrowsingContext, | |||
| accepted: bool, | |||
| type: "alert" / "confirm" / "prompt" / "beforeunload", | |||
There was a problem hiding this comment.
nit: perhaps it makes sense to extract this into a separate type and re-use between UserPromptClosedParameters and UserPromptOpenedParameters to make sure they are always the same?
The key part of this is that with BiDi HTML calls into the spec when it's going to show a prompt. So at that point we can see if there's a handler defined, and if so return the handler to HTML. All the actual logic to handle the dialogs ends up in the HTML spec. From the BiDi point of view, the intended lifecycle is that you always get the session.userPromptOpened and session.userPromptClosed events, even if the prompt is automatically handled. But the session.UserPromptOpened event gets a new `handler` property that tells you whether the prompt will be automatically handled; if this is set to "none" the automation has to send a session.handleUserPrompt command (or wait for the prompt to be dismissed by the user).
Preview | Diff