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

Implement input.FileDialogOpened #568

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Implement input.FileDialogOpened #568

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3f0bb48
Implement `input.FileDialogOpened`
jrandolf-2 Oct 9, 2023
4e766f5
Add capability processing
OrKoN Nov 21, 2023
ef7baa7
Added multiple
OrKoN Nov 27, 2023
dc921f7
Mention default value.
OrKoN Dec 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 100 additions & 0 deletions index.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ spec: HTML; urlPrefix: https://html.spec.whatwg.org/multipage/
text: innerText getter steps; url:dom.html#dom-innertext
text: navigables; url: document-sequences.html#navigables
text: navigation id; url: browsing-the-web.html#navigation-id
text: multiple; url: input.html#attr-input-multiple
text: origin-clean; url: canvas.html#concept-canvas-origin-clean
text: parent; for:navigable; url: document-sequences.html#nav-parent
text: prompt to unload; url: browsing-the-web.html#prompt-to-unload-a-document
Expand Down Expand Up @@ -1400,6 +1401,7 @@ for a session.
<pre class="cddl remote-cddl local-cddl">
session.CapabilityRequest = {
? acceptInsecureCerts: bool,
? interceptFileDialogs: bool,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

? browserName: text,
? browserVersion: text,
? platformName: text,
Expand Down Expand Up @@ -1444,6 +1446,34 @@ with parameter |value| is:

</div>

<pre class=simpledef>
Capability: <dfn export>intercept file dialogs</dfn>
Key: "<code>interceptFileDialogs</code>"
Value type: boolean
Description: Defines whether the file dialogs should be intercepted and not shown on screen. Defaults to false.
</pre>

<div algorithm="interceptFileDialogs capability deserialization algorithm">
The [=additional capability deserialization algorithm=] for the
"<code>interceptFileDialogs</code>" capability, with parameter |value| is:

1. If |value| is not a boolean, return [=error=] with [=error code|code=]
[=invalid argument=].

1. Return [=success=] with data |value|.

</div>

<div algorithm="interceptFileDialogs capability serialization algorithm">
The [=matched capability serialization algorithm=] for the "<code>interceptFileDialogs</code>" capability,
with parameter |value| is:

1. If |value| is false, return [=success=] with data null.
OrKoN marked this conversation as resolved.
Show resolved Hide resolved

1. Return [=success=] with data true.
OrKoN marked this conversation as resolved.
Show resolved Hide resolved

</div>

#### The session.ProxyConfiguration Type #### {#type-session-ProxyConfiguration}

[=remote end definition=] and [=local end definition=]
Expand Down Expand Up @@ -9589,6 +9619,76 @@ The [=remote end steps=] given |session|, and |command parameters| are:

</div>

### Events ### {#module-input-events}

#### The input.fileDialogOpened Event #### {#event-input-fileDialogOpened}

<dl>
<dt>Event Type</dt>
<dd>
<pre class="cddl local-cddl remote-cddl">
input.FileDialogOpened = (
method: "input.fileDialogOpened",
params: input.FileDialogInfo
)

input.FileDialogInfo = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we provide an info if the file dialog was intercepted or not? And if so, should the value be different for different sessions depending on the intercept file dialogs capability?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dialog is a shared resource so it will be intercepted if any of the active session intercepts it. I think a flag would be useful but there is no immediate use case for it.

context: browsingContext.BrowsingContext,
element: script.SharedReference,
multiple: bool,
}
</pre>
</dd>
</dl>

<div algorithm>

The [=remote end event trigger=] is the <dfn export>WebDriver BiDi file dialog
opened</dfn> steps given |element|.

1. Let |context| be the |element|'s [=node document=]'s [=/browsing context=].

1. Let |context id| be the [=browsing context id=] for |context|.

1. Let |related browsing contexts| be a [=/set=] containing |context|.

1. Let |sessions| be the [=set of sessions for which an event is enabled=]
given <code>"input.fileDialogOpened"</code> and |related browsing
contexts|.

1. Let |params| be a [=/map=] matching the <code>input.FileDialogInfo</code>
production, with the <code>context</code> field set to |context id|, the
<code>element</code> field set to |element|, and the <code>multiple</code>
field set to true if the [=multiple=] attribute is set on |element| and false
otherwise.

1. Let |body| be a [=/map=] matching the <code>input.FileDialogOpened</code>
production, with the <code>params</code> field set to |params|.

1. For each |session| in |sessions|:

1. [=Emit an event=] with |session| and |body|.

1. Let |intercepting sessions| be the [=set of sessions for which file dialog interception is enabled=].
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this should use "did any session suppress the dialog". Rather if any session actually requests a specific capability value here then any later session requesting a different capability value should fail during session creation. If sessions don't request a specific value then a later session requesting a specific value can affect the behaviour (maybe later we should have a session.CapabilityChanged event for when a new session affects existing settings, but that's out of scope for now).


1. Return false if |intercepting sessions| is empty, and true otherwise.

</div>

<div algorithm>

The <dfn>set of sessions for which file dialog interception is enabled</dfn> is:

1. Let |sessions| be a new [=/set=].

1. For each |session| in [=active BiDI sessions=]:

1. If |session|'s [=intercept file dialogs=] capability is true, append |session| to |sessions|.

1. Return |sessions|.

</div>

# Patches to Other Specifications # {#patches}

This specification requires some changes to external specifications to provide the necessary
Expand Down