Skip to content

Special-case <dialog> element [open] attribute - #165

Open
seanpdoyle wants to merge 1 commit into
bigskysoftware:mainfrom
seanpdoyle:dialog-close-remove-open
Open

seanpdoyle wants to merge 1 commit into
bigskysoftware:mainfrom
seanpdoyle:dialog-close-remove-open

Conversation

@seanpdoyle

@seanpdoyle seanpdoyle commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

The HTML specification explicitly highlights the risk of removing the [open] attribute directly:

Removing the open attribute will usually hide the dialog. However, doing so has a number of strange additional consequences:

The close event will not be fired.

The close() method, and any close requests, will no longer be able to close the dialog.

If the dialog was shown using its showModal() method, the Document will still be blocked.

For these reasons, it is generally better to never remove the open attribute manually. Instead, use the requestClose() or close() methods to close the dialog, or the hidden attribute to hide it.

--

Similarly, according to the MDN documentation for the HTMLDialogElement.open() attribute:

Warning: While the open property is technically not read-only and can
be set directly, doing so is strongly discouraged by the HTML
specification, as it can break normal dialog interactions in unexpected
ways. For example, the close event won't fire when programmatically
setting open to false, and subsequent calls to the close() and
requestClose() methods will have no effect. Instead, it's better to
use methods such as show(), showModal(), close(), and
requestClose() to change the value of the open attribute.

-- MDN documentation for HTMLDialogElement.open property value

Prior to this commit, Idiomorph.morph invocations that resulted in the removal of the [open] attribute violated this guidance. If the [open] attribute was present (regardless of its provenance: HTMLDialogElement.show(), HTMLDialogElement.showModal(), or being rendered with [open]), the morph would remove the attribute itself in a way that circumvents the internal state management of the <dialog> element.

This commit introduces a special-case conditional that skips the explicit removal of the [open] in favor of invoking HTMLDialogElement.close() directly. Since morphs occur without end-user involvement, .close() is invoked rather than HTMLDialogElement.requestClose().

According to the MDN documentation for the [HTMLDialogElement.open()][]
attribute:

> *Warning*: While the open property is technically not read-only and can
> be set directly, doing so is strongly discouraged by the HTML
> specification, as it can break normal dialog interactions in unexpected
> ways. For example, the `close` event won't fire when programmatically
> setting `open` to `false`, and subsequent calls to the `close()` and
> `requestClose()` methods will have no effect. Instead, it's better to
> use methods such as `show()`, `showModal()`, `close()`, and
> `requestClose()` to change the value of the `open` attribute.
>
> -- MDN documentation for `HTMLDialogElement.open` property value

Prior to this commit, `Idiomorph.morph` invocations that resulted in the
removal of the `[open]` attribute violated this guidance. If the
`[open]` attribute was present (regardless of its provenance:
`HTMLDialogElement.show()`, `HTMLDialogElement.showModal()`, or being
rendered with `[open]`), the morph would remove the attribute itself in
a way that circumvents the internal state management of the `<dialog>`
element.

This commit introduces a special-case conditional that skips the
explicit removal of the `[open]` in favor of invoking
[HTMLDialogElement.close()][] directly. Since morphs occur without
end-user involvement, `.close()` is invoked rather than
[HTMLDialogElement.requestClose()][].

[HTMLDialogElement.open()]: https://developer.mozilla.org/en-US/docs/Web/API/HTMLDialogElement/open#value
[HTMLDialogElement.close()]: https://developer.mozilla.org/en-US/docs/Web/API/HTMLDialogElement/close
[HTMLDialogElement.requestClose()]: https://developer.mozilla.org/en-US/docs/Web/API/HTMLDialogElement/requestClose
@seanpdoyle

Copy link
Copy Markdown
Contributor Author

@botandrose This proposal is motivated by hotwired/turbo#1239 and hotwired/turbo#1592. Turbo is currently capable of owning this behavior, but given the explicit guidance provided by the MDN documentation, there is an opportunity to encode this decision making process directly into Idiomorph itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant