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

Add request-close command for dialogs #11045

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
+67 −28
Open

Add request-close command for dialogs #11045

Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
830339d
Add request-close command for dialogs
lukewarlow Feb 19, 2025
0b2bccb
Address comments
lukewarlow Feb 25, 2025
990132e
lowercase dialog
lukewarlow Feb 26, 2025
26c9782
Address comments
lukewarlow Mar 3, 2025
627ea99
Update request-close and close command handling of button value attri…
lukewarlow Mar 3, 2025
82c7bd0
Address comment
lukewarlow Mar 3, 2025
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
85 changes: 60 additions & 25 deletions source
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53560,6 +53560,11 @@ interface <dfn interface>HTMLButtonElement</dfn> : <span>HTMLElement</span> {
data-x="attr-button-command-close"><code>close</code></dfn>
<td><dfn data-x="attr-button-command-close-state">Close</dfn>
<td>Closes the targeted <code>dialog</code> element.
<tr>
<td><dfn attr-value for="html-global/command"
data-x="attr-button-command-request-close"><code>request-close</code></dfn>
<td><dfn data-x="attr-button-command-request-close-state">Request Close</dfn>
<td>Requests to close the targeted <code>dialog</code> element.
<tr>
<td><dfn attr-value for="html-global/command"
data-x="attr-button-command-show-modal"><code>show-modal</code></dfn>
Expand Down Expand Up @@ -62226,25 +62231,10 @@ interface <dfn interface>HTMLDialogElement</dfn> : <span>HTMLElement</span> {
are:</p>

<ol>
<li><p>If <span>this</span> does not have an <code data-x="attr-dialog-open">open</code>
attribute, then return.</p></li>

<li><p><span>Assert</span>: <span>this</span>'s <span data-x="dialog-close-watcher">close
watcher</span> is not null.</p></li>

<li><p>Set <var>dialog</var>'s <span>enable close watcher for <code
data-x="dom-dialog-requestClose">requestClose()</code></span> to true.</p></li>

<li><p>If <var>returnValue</var> is not given, then set it to null.</p></li>
Copy link
Member

Choose a reason for hiding this comment

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

Why do we set it to null and not the empty string? With button elements it can never become null but that doesn't seem to matter as null and the empty string are always handled identically in close the dialog? So maybe we should remove that distinction? Probably okay as a follow-up, though might be nice to do here as now we're adding additional callers.

Copy link
Member Author

@lukewarlow lukewarlow Mar 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

null and the empty string aren't handled identically. If it's an empty string it replaces returnValue, if it's null returnValue isn't set to the new value (so keeps it's old value). Step 10 https://html.spec.whatwg.org/multipage/interactive-elements.html#close-the-dialog

Copy link
Member

Choose a reason for hiding this comment

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

Was there no desire for this new feature to be able to emulate that then? Should this not be called out somehow?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think you've found a spec issue here, because Chrome does distinguish between button with a value attribute and button without. I'm just looking into it now.

https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#concept-form-submit - the existing place does "if submitter has a value", is that correct spec language that I can copy? I don't know if has a value is really defined?

Copy link
Member

Choose a reason for hiding this comment

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

A button element is defined to always have a value. See its concept-fe-value definition.

Copy link
Member Author

Choose a reason for hiding this comment

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

Turns out there's an interop issue here, WebKit and Chrome behave different to Firefox (which follows the spec)

Copy link
Member Author

@lukewarlow lukewarlow Mar 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I've updated the spec for close and request-close commands to check for the attributes existence before using the value. I'm checking WPTs for command invokers now to see if this case is covered if not I'll add them to the linked test PR. I'll raise a separate issue for the dialog form case: #11092

Copy link
Member Author

Choose a reason for hiding this comment

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

Have added tests to that PR.

Copy link
Member

Choose a reason for hiding this comment

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

then set*

But if we have to do this for all consumers of concept-fe-value maybe it should be defined differently. Hmm.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah perhaps as part of #11092 we'll need to revise how this is written.


<li><p>Set <span>this</span>'s <span>request close return value</span> to
<var>returnValue</var>.</p></li>

<li><p><span data-x="close-watcher-request-close">Request to close</span> <var>dialog</var>'s
<span data-x="dialog-close-watcher">close watcher</span> with false.</p></li>

<li><p>Set <var>dialog</var>'s <span>enable close watcher for <code
data-x="dom-dialog-requestClose">requestClose()</code></span> to false.</p></li>
<li><p><span data-x="dialog-request-close">Request to close the dialog</span> <span>this</span>
with <var>returnValue</var>.</p></li>
</ol>

<div class="note" id="note-dialog-method-names">
Expand Down Expand Up @@ -62320,8 +62310,14 @@ interface <dfn interface>HTMLDialogElement</dfn> : <span>HTMLElement</span> {
<p>Each <code>dialog</code> element has a <dfn>request close return value</dfn>, which is a
string, initially null.</p>

<p>Each <code>dialog</code> element has an <dfn>enable close watcher for <code
data-x="dom-dialog-requestClose">requestClose()</code></dfn> boolean, initially false.</p>
<p>Each <code>dialog</code> element has an <dfn>enable close watcher for request close</dfn>
boolean, initially false.</p>

<p class="note"><code>dialog</code>'s <span>enable close watcher for request close</span> is used
to force enable the dialog's <span data-x="dialog-close-watcher">close watcher</span> while <span
data-x="dialog-request-close">requesting to close</span> it. A dialog whose <span>computed
closed-by state</span> is the <span data-x="attr-dialog-closedby-none-state">None</span> state
would otherwise have a disabled <span data-x="dialog-close-watcher">close watcher</span>.</p>

<p>Each <code>dialog</code> element has an <dfn>is modal</dfn> boolean, initially false.</p>

Expand Down Expand Up @@ -62468,9 +62464,8 @@ interface <dfn interface>HTMLDialogElement</dfn> : <span>HTMLElement</span> {
value</span>.</p></li>

<li><p><i data-x="create-close-watcher-getEnabledState">getEnabledState</i> being to return
true if <var>dialog</var>'s <span>enable close watcher for <code
data-x="dom-dialog-requestClose">requestClose()</code></span> is true or <var>dialog</var>'s
<span>computed closed-by state</span> is not <span
true if <var>dialog</var>'s <span>enable close watcher for request close</span> is true or
<var>dialog</var>'s <span>computed closed-by state</span> is not <span
data-x="attr-dialog-closedby-none-state">None</span>; otherwise false.</p></li>
</ul>
</li>
Expand All @@ -62481,9 +62476,9 @@ interface <dfn interface>HTMLDialogElement</dfn> : <span>HTMLElement</span> {

<ol>
<li><p>If <var>command</var> is in the <span
data-x="attr-button-command-close-state">Close</span> state or in the <span
data-x="attr-button-command-show-modal-state">Show Modal</span> state, then
return true.</p></li>
data-x="attr-button-command-close-state">Close</span> state, <span
data-x="attr-button-command-request-close-state">Request Close</span> state, or the <span
data-x="attr-button-command-show-modal-state">Show Modal</span> state, then return true.</p></li>

<li><p>Return false.</p></li>
</ol>
Expand All @@ -62508,6 +62503,20 @@ interface <dfn interface>HTMLDialogElement</dfn> : <span>HTMLElement</span> {
</ol>
</li>

<li>
<p>If <var>command</var> is in the <span
data-x="attr-button-command-request-close-state">Request Close</span> state and
<var>element</var> has an <code data-x="attr-dialog-open">open</code> attribute:</p>

<ol>
<li><p>Let <var>value</var> be <var>invoker</var>'s <span
data-x="concept-fe-value">value</span>.</p></li>

<li><p><span data-x="dialog-request-close">Request to close the dialog</span>
<var>element</var> with <var>value</var>.</p></li>
</ol>
</li>

<li><p>If <var>command</var> is the <span
data-x="attr-button-command-show-modal-state">Show Modal</span> state and <var>element</var> does
not have an <code data-x="attr-dialog-open">open</code> attribute, then
Expand Down Expand Up @@ -62610,6 +62619,31 @@ interface <dfn interface>HTMLDialogElement</dfn> : <span>HTMLElement</span> {
</li>
</ol>

<p>To <dfn data-x="dialog-request-close">request to close</dfn> <code>dialog</code> element
<var>subject</var>, given null or a string <var>returnValue</var>:</p>

<ol>
<li><p>If <span>this</span> does not have an <code data-x="attr-dialog-open">open</code>
attribute, then return.</p></li>

<li><p><span>Assert</span>: <span>this</span>'s <span data-x="dialog-close-watcher">close
watcher</span> is not null.</p></li>

<li><p>Set <var>dialog</var>'s <span>enable close watcher for request close</span> to
true.</p></li>

<li><p>If <var>returnValue</var> is not given, then set it to null.</p></li>

<li><p>Set <span>this</span>'s <span>request close return value</span> to
<var>returnValue</var>.</p></li>

<li><p><span data-x="close-watcher-request-close">Request to close</span> <var>dialog</var>'s
<span data-x="dialog-close-watcher">close watcher</span> with false.</p></li>

<li><p>Set <var>dialog</var>'s <span>enable close watcher for request close</span> to
false.</p></li>
</ol>

</div>

<p>To <dfn>queue a dialog toggle event task</dfn> given a <code>dialog</code> element
Expand Down Expand Up @@ -143435,6 +143469,7 @@ interface <dfn interface>External</dfn> {
"<code data-x="attr-button-command-show-popover">show-popover</code>";
"<code data-x="attr-button-command-hide-popover">hide-popover</code>";
"<code data-x="attr-button-command-close">close</code>";
"<code data-x="attr-button-command-request-close">request-close</code>";
"<code data-x="attr-button-command-show-modal">show-modal</code>";
a <span data-x="attr-button-command-custom">custom command keyword</span>
<tr>
Expand Down