[WIP] Implement `input.fileDialogOpened` #568

jrandolf-2 · 2023-10-09T12:37:26Z

This PR implements the event portion of file dialog flow.

Relevant HTML spec: whatwg/html#11030
Relevant WebDriver Classic spec: w3c/webdriver#1884
Issue: #494

Preview | Diff

jgraham

So, does this require a patch in HTML at https://html.spec.whatwg.org/#the-input-element-as-a-file-upload-control ?

How are you imagining the return value would be used?

jgraham · 2023-10-27T14:23:04Z

Oh, sorry I missed that there is an HTML patch, I'll read that :)

jgraham · 2023-10-27T15:04:33Z

So generally a concern I have at the moment is that we don't have any mechanism to specify the behaviour when the dialog is intercepted. I slightly worry that the default here being different to the default for alerts is confusing (with alerts we actually show the alert, even if there's an event, and rely on the subscriber sending a command to continue. If possible I think we ought to do that by default in this case as well, but also make it configurable in some way).

jrandolf-2 · 2023-10-28T00:35:42Z

@jgraham The problem with that approach is the blocking behavior of file dialogs, so you cannot set multiple file choosers in parallel.

IIRC, there was a consensus in TPAC to not allow that blocking behavior for file dialogs. Perhaps @OrKoN can confirm.

OrKoN · 2023-10-30T08:46:21Z

So generally a concern I have at the moment is that we don't have any mechanism to specify the behaviour when the dialog is intercepted. I slightly worry that the default here being different to the default for alerts is confusing (with alerts we actually show the alert, even if there's an event, and rely on the subscriber sending a command to continue. If possible I think we ought to do that by default in this case as well, but also make it configurable in some way).
IIRC, there was a consensus in TPAC to not allow that blocking behavior for file dialogs.

I think we touched upon that at TPAC and the idea is that file dialogs might not actually block the execution on the page so it is possible for the user to continue doing things (in the background via scripts and trigger other dialogs) while the dialog is showing that might have unpredictable results. It is different from the alerts where the loop is actually blocked while waiting for the alert resolution. We also discussed that some user agents might not implement a dialog at all.
So, the current version would be preferred from the Puppeteer's perspective.

@jimevans @shs96c do you have insights into what would be preferred by Selenium? I feel like if the client opts into intercepting the dialogs programmatically, there might be no need to show it on screen. WDYT?

jgraham · 2023-10-30T09:37:09Z

Conceptually file dialogs are similar to alert-type prompts, so I'm keen to make sure we don't end up with different behaviour in each case.

With alerts we currently send the client an event, and you're required to respond with a command to dismiss the alert (and provide the return value for the types where that makes sense).

The proposal here is that with file prompts opting in to getting the event means that you don't get a prompt at all, but only get an event, and can provide the value asynchronously if required. Technically that does make some sense (an alert is fundamentally synchronous, whereas you can provide an input to a file dialog at any time), but it's not clear whether or not that difference is enough to justify having a totally different model compared to alerts.

I'm also not sure about just subscribing to an event having side effects on browser behaviour. It is true that with e.g. network request interception we don't block the request if it matches a pattern but there isn't any session with a subscription to the relevant event. So there is some precedent there. But it also seems meaningfully different that interception is itself a webdriver feature, and without someone subscribing to the event it would be possible for anyone (even a human interacting with the browser) to unblock the request.

lukewarlow · 2023-11-11T22:58:30Z

Why does this only intercept the show picker algorithm for file inputs? Seems like it would be useful to provide a way to intercept all the show picker algorithms?

jrandolf-2 · 2023-11-16T12:11:39Z

Why does this only intercept the show picker algorithm for file inputs? Seems like it would be useful to provide a way to intercept all the show picker algorithms?

For now, we are limiting the scope of this PR to file inputs.

OrKoN · 2023-11-21T14:05:49Z

I have updated the PR to include the capability processing to reflect the latest discussion at the working group. PTAL

whimboo · 2023-11-24T08:33:55Z

index.bs

@@ -1400,6 +1400,7 @@ for a session.
 <pre class="cddl remote-cddl local-cddl">
 session.CapabilityRequest = {
  ? acceptInsecureCerts: bool,
+  ? interceptFileDialogs: bool,


By having such a capability I wonder how we best separate those which belong to different protocols. Right now some of the capabilities that we have are used in both classic and bidi, but others are classic only. Now we get bidi only capabilities. All that doesn't make it easy. I wonder if we should actually re-use unhandledPromptBehavior for bidi as well, or if we want to make the dialog handling dependent on subscribed events.

I think it's already separated because the BiDi spec defines these as additional capabilities. Or do you mean how to separate them in the CDDL definitions?

@whimboo @jgraham What do you think? The concept of “additional WebDriver capabilities” is defined in the Classic spec, but the WebDriver BiDi spec makes it clear that these additional WebDriver capabilities in particular are BiDi-only:

WebDriver BiDi defines additional WebDriver capabilities. The following tables enumerates the capabilities each implementation must support for WebDriver BiDi.

Do you have a suggestion of a better way to phrase this?

I don't think that BiDi-only capabilities are a problem in general.

Currently BiDi doesn't use unhandledPromptBehavior at all, but I assume that will change.

If I was designing this from scratch, I might have something like:

session.CapabilityRequest = { … promptBehavior: session.PromptBehavior } session.PromptBehavior = { ?alert: : session.PromptBehaviorOption, ?confirm: : session.PromptBehaviorOption, ?default: session.PromptBehaviorOption .default "ignore", ?file: : session.PromptBehaviorOption, ?httpAuth: : session.PromptBehaviorOption, ?unload: : session.PromptBehaviorOption, } session.PromptBehaviorOption = "accept" / "dismiss" / "ignore"

Then for each prompt type you could have different behaviour, depending on what you expect without needing a separate top-level capability. Classic's unhandledPromptBehavior would be more or less equivalent to {default: <behavior>}. For compatibility we could always use unhandledPromptBehavior as a fallback if nothing else is specified.

It does not look like unhandledPromptBehavior in classic includes file dialogs (https://html.spec.whatwg.org/#user-prompts)? So it would not be exactly equivalent to the default behavior. Should we change to this structure as part of this PR? I am not sure how would the accept and dismiss work as I understood it's rather difficult to dismiss file dialogs also in Firefox (based on a thread in Matrix)

Right, probably only "dismiss" and "ignore" work for all dialogs. "accept" works with dialogs which don't take a value or for which there's a reasonable default value. That doesn't include file or HTTP Auth.

I am also not sure if dismiss works for files because we don't actually want to dismiss the dialog (e.g., dispatch the cancel event on element) but rather control whether it is shown on screen or not. So do you suggest we introduce the general promptBehavior capability for this feature or does a standalone capability (current version) sound okay for this?

I think we should introduce something that we can extend to cover more dialogs, even if for now we only support it for file dialogs. So a definition like:

session.CapabilityRequest = { … promptBehavior: session.PromptBehavior } session.PromptBehavior = { ?file: : "suppress" / "dismiss" / "none", }

would be fine. "suppress" would emit the event but not display the dialog. "dismiss" would cause a cancel event to be fired, as if the Cancel button was pressed, and "none" would not affect the dialog (although an event would still be fired if a session was subscribed). I"d even be happy to drop "dismiss" for now, which I think should make it functionally equivalent to what you already have.

index.bs

jgraham · 2023-11-27T13:23:49Z

index.bs

@@ -1400,6 +1400,7 @@ for a session.
 <pre class="cddl remote-cddl local-cddl">
 session.CapabilityRequest = {
  ? acceptInsecureCerts: bool,
+  ? interceptFileDialogs: bool,


I don't think that BiDi-only capabilities are a problem in general.

Currently BiDi doesn't use unhandledPromptBehavior at all, but I assume that will change.

If I was designing this from scratch, I might have something like:

session.CapabilityRequest = { … promptBehavior: session.PromptBehavior } session.PromptBehavior = { ?alert: : session.PromptBehaviorOption, ?confirm: : session.PromptBehaviorOption, ?default: session.PromptBehaviorOption .default "ignore", ?file: : session.PromptBehaviorOption, ?httpAuth: : session.PromptBehaviorOption, ?unload: : session.PromptBehaviorOption, } session.PromptBehaviorOption = "accept" / "dismiss" / "ignore"

Then for each prompt type you could have different behaviour, depending on what you expect without needing a separate top-level capability. Classic's unhandledPromptBehavior would be more or less equivalent to {default: <behavior>}. For compatibility we could always use unhandledPromptBehavior as a fallback if nothing else is specified.

index.bs

sadym-chromium

LGTM. The isIntercepted flag in the event can be added later on.

css-meeting-bot · 2024-09-27T19:26:51Z

The Browser Testing and Tools Working Group just discussed Implement input.FileDialogOpened.

The full IRC log of that discussion

<AutomatedTester> topic: Implement input.FileDialogOpened
<AutomatedTester> github: https://github.com//pull/568
<AutomatedTester> sadym: I need to go review this and get back to the WG

whimboo · 2025-02-13T08:47:05Z

sadym: I need to go review this and get back to the WG

@sadym-chromium would you mind to follow-up on this action item? Thanks!

sadym-chromium · 2025-02-25T10:24:25Z

index.bs

+   1. Otherwise if |user prompt handler| [=map/contains=] "<code>default</code>" and
+      |user prompt handler|["<code>fileDialog</code>"] is not equal to
+      "<code>ignore</code>", set |dismissed| to true.


Respecting default behavior makes sense, but it is a breaking change, as default of "accept" and "dismiss" starts suppressing file dialog.

+ remove picker type + check prompt handler through all the sessions

sadym-chromium · 2025-02-25T11:02:57Z

index.bs

@@ -2282,7 +2287,7 @@ BrowserResult = (

 Each [=/top-level traversable=] is associated with a single <dfn>client
 window</dfn> which represents a rectangular area containing the
-<a spec=css22>viewport</a> that will be used to render that [=/top-level
+<a spec=css2>viewport</a> that will be used to render that [=/top-level


My local build didn't work with css22 for whatever reason.

sadym-chromium · 2025-02-25T12:09:34Z

Closing in favor of #883

jrandolf-2 requested a review from OrKoN October 9, 2023 12:37

jrandolf-2 force-pushed the jrandolf/file-dialog-opened branch 3 times, most recently from b8247dc to 3bd59bb Compare October 9, 2023 13:13

OrKoN approved these changes Oct 9, 2023

View reviewed changes

jrandolf-2 requested review from whimboo and jgraham October 9, 2023 13:43

jgraham reviewed Oct 27, 2023

View reviewed changes

OrKoN mentioned this pull request Oct 30, 2023

Add input.SetFiles #514

Merged

jrandolf-2 force-pushed the jrandolf/file-dialog-opened branch from 3bd59bb to e31b876 Compare November 8, 2023 11:34

jrandolf-2 requested a review from jgraham November 10, 2023 10:55

OrKoN force-pushed the jrandolf/file-dialog-opened branch 2 times, most recently from 5421b69 to 4e766f5 Compare November 21, 2023 14:00

OrKoN self-requested a review November 21, 2023 14:12

whimboo reviewed Nov 24, 2023

View reviewed changes

OrKoN requested a review from sadym-chromium November 27, 2023 12:22

jgraham reviewed Nov 27, 2023

View reviewed changes

OrKoN requested a review from gsnedders December 7, 2023 09:27

OrKoN approved these changes Dec 7, 2023

View reviewed changes

sadym-chromium reviewed Dec 8, 2023

View reviewed changes

index.bs Outdated Show resolved Hide resolved

sadym-chromium reviewed Dec 8, 2023

View reviewed changes

index.bs Show resolved Hide resolved

sadym-chromium approved these changes Dec 8, 2023

View reviewed changes

sadym-chromium changed the title ~~Implement input.FileDialogOpened~~ Implement input.fileDialogOpened Feb 17, 2025

sadym-chromium force-pushed the jrandolf/file-dialog-opened branch from dc921f7 to 6ea3e13 Compare February 17, 2025 14:56

sadym-chromium changed the title ~~Implement input.fileDialogOpened~~ [WIP] Implement input.fileDialogOpened Feb 17, 2025

sadym-chromium marked this pull request as draft February 17, 2025 15:35

sadym-chromium mentioned this pull request Feb 24, 2025

Allow file prompt type w3c/webdriver#1884

Merged

sadym-chromium reviewed Feb 25, 2025

View reviewed changes

sadym-chromium force-pushed the jrandolf/file-dialog-opened branch from b6ba3ac to 3e560a8 Compare February 25, 2025 10:56

sadym-chromium added 13 commits February 25, 2025 12:00

WebDriver BiDi picker shown + input.fileDialogOpened

c65f82b

todo

d41574f

Extend capabilities

1802ae1

Explain behavior

689694a

rename capability to fileUpload

b1a76df

Suppress file dialog

e2be155

Return pair (|suppressed|, |dismissed|)

bf64757

remove suppress file dialog

8accc43

update note

0610177

Rename to "WebDriver BiDi file dialog shown"

44ec72c

+ remove picker type + check prompt handler through all the sessions

fix arguments

64ab989

"WebDriver BiDi file dialog opened"

be8a54e

fix css2 reference

5b8ff04

sadym-chromium force-pushed the jrandolf/file-dialog-opened branch from 3e560a8 to 5b8ff04 Compare February 25, 2025 11:00

sadym-chromium reviewed Feb 25, 2025

View reviewed changes

sadym-chromium added 3 commits February 25, 2025 12:08

fix loop

1d7bf4b

fix condition

935699b

Rename capability to file

0b4f673

sadym-chromium mentioned this pull request Feb 25, 2025

input.fileDialogOpened + unhandledPromptBehavior:file #883

Merged

sadym-chromium closed this Feb 25, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Implement `input.fileDialogOpened` #568

[WIP] Implement `input.fileDialogOpened` #568

jrandolf-2 commented Oct 9, 2023 •

edited by pr-preview bot

Loading

jgraham left a comment

jgraham commented Oct 27, 2023

jgraham commented Oct 27, 2023

jrandolf-2 commented Oct 28, 2023

OrKoN commented Oct 30, 2023

jgraham commented Oct 30, 2023 •

edited

Loading

lukewarlow commented Nov 11, 2023

jrandolf-2 commented Nov 16, 2023

OrKoN commented Nov 21, 2023

whimboo Nov 24, 2023

OrKoN Nov 24, 2023

mathiasbynens Nov 27, 2023

jgraham Nov 27, 2023

OrKoN Nov 27, 2023

jgraham Nov 27, 2023

OrKoN Nov 27, 2023

jgraham Jan 10, 2024

jgraham Nov 27, 2023

sadym-chromium left a comment

css-meeting-bot commented Sep 27, 2024

whimboo commented Feb 13, 2025

sadym-chromium Feb 25, 2025 •

edited

Loading

sadym-chromium Feb 25, 2025

sadym-chromium commented Feb 25, 2025

[WIP] Implement input.fileDialogOpened #568

[WIP] Implement input.fileDialogOpened #568

Conversation

jrandolf-2 commented Oct 9, 2023 • edited by pr-preview bot Loading

jgraham left a comment

Choose a reason for hiding this comment

jgraham commented Oct 27, 2023

jgraham commented Oct 27, 2023

jrandolf-2 commented Oct 28, 2023

OrKoN commented Oct 30, 2023

jgraham commented Oct 30, 2023 • edited Loading

lukewarlow commented Nov 11, 2023

jrandolf-2 commented Nov 16, 2023

OrKoN commented Nov 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sadym-chromium left a comment

Choose a reason for hiding this comment

css-meeting-bot commented Sep 27, 2024

whimboo commented Feb 13, 2025

sadym-chromium Feb 25, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sadym-chromium commented Feb 25, 2025

[WIP] Implement `input.fileDialogOpened` #568

[WIP] Implement `input.fileDialogOpened` #568

jrandolf-2 commented Oct 9, 2023 •

edited by pr-preview bot

Loading

jgraham commented Oct 30, 2023 •

edited

Loading

sadym-chromium Feb 25, 2025 •

edited

Loading