Description
What is the issue with the HTML Standard?
Right now the spec has some fairly wide holes when it comes to requestClose
on HTMLDialogElement
. As it stands the spec for requestClose steps asserts that the dialog's CloseWatcher
is not null, but it can be - fairly trivially so - as the only times CloseWatcher
is established are during show()
and showModal()
. This overlaps quite closely with #10982 and #10953.
Consequently @lukewarlow raised #10954 which adds HTML Element insertion steps and Attribute change steps to Dialog elements which, if the open attribute is present, will also set up the CloseWatcher
. However in some cases; for example a disconnected tree or a document without a browsing context, the CloseWatcher
may or may not be established, or other asserts may fail. We cans see this in web-platform-tests/wpt#52020 which tests both of these conditions, and currently causes crashes on Chrome.
Taking a step back slightly, when we look at the interaction requestClose
has with CloseWatcher
, it's not entirely apparent why requestClose
needs CloseWatcher
. The "get enabled state" machinery in CloseWatcher
was added so that requestClose
would work if Dialog's computed closed-by state was None - allowing Dialog requestClose
to still work even if the CloseWatcher
was disabled. Additionally CloseWatcher
's "request to close" algorithm takes a requireHistoryActionActivation
boolean, which for light dismiss and requestClose
circumvents the CloseWatcher protections around ensuring the cancel
event cannot be default prevented. In other words for light dismiss and requestClose
can always be default prevented, which also means they never consume a history-user activation.
So to summarise we have a few problems: requestClose & light dismiss have added additional machinery that sometimes disables relevant parts of CloseWatcher but hasn't yet gone far enough to cover edge cases that CloseWatcher is active for the dialog element.
I think we can look at solving this problem in a few ways, but each has some downsides:
- Remove asserts that Dialog should have an active close watcher, instead creating one if it doesn't exist.
- This has issues in that construction of a close watcher may create a new group, so there can be observable differences if you call requestClose on a dialog that was open before it was connected rather than after.
- Remove the CloseWatcher mechanics entirely from
requestClose
, and have it dispatchcancel
/close
events by itself, without expecting or interacting with the CloseWatcher.- This is potentially problematic because
requestClose
should closely match whatCloseWatcher.requestClose
does - so the two need to be kept in sync, or we need to abstract both algorithms out into a third, which will likely inherit the complexity they already have. This isn't just spec purity but also implementation complexity.
- This is potentially problematic because
- "Double down" on new mechanics to solve these edge cases, such as incorporating the changes in Fix dialog closedby and requestClose() #10954 and well beyond - including ensuring that CloseWatchers are only sometimes created during insertion/attribute changes.
- This has some of the same problems as 1 though they're even more subtle, but also some of the same problems as 2 in terms of spec/implementation complexity.