Close watchers: detecting close requests vs. other closes

[domenic]

What is the issue with the HTML Standard?

In https://bugs.chromium.org/p/chromium/issues/detail?id=1512224, especially this comment, I was made aware that developers were using <dialog>'s cancel event as a way of detecting when a close request was issued vs. whether the dialog was closed in some other way.

This conflicts a bit with the changes introduced in #9462. There, the close watcher infrastructure only fires cancel if it's preventDefault()-able, so that web developers can call cancelEvent.preventDefault() and stop the dialog or close watcher from closing. When designing that infrastructure, I didn't realize that people might be using cancel for this sort of informational purpose; I thought they would only use it to intervene.

To accomplish this use case, of detecting close requests, two solutions come to mind:

• Add a property to the close event, e.g. fromCloseRequest or userInitiated or something.
• Fire cancel anyway, but make cancelEvent.cancelable false when the anti-abuse protections do not allow calling preventDefault(). This automatically makes it so that cancelEvent.preventDefault() does nothing.

For both options, we have the choice of whether to apply them just to <dialog>, or to apply them also to CloseWatcher.

I'm currently leaning toward the second option, as it does not require web developers to change their code. However, based on our measurements from shipping #9462, only 0.000015% of page views experienced a change in the number of cancel events fired, so it isn't required that we take that path. (And, after fixing #10046, that 0.000015% number will go even lower.)

My plan is to write up the spec changes for "fire cancel anyway", for both <dialog> and CloseWatcher, and see how it feels. If it's very ugly or there's some unintended consequence that I didn't understand, I'll revisit and consider "add a property to the close event".

/cc @josepharhar @lukewarlow

[domenic]

Hmm, I forgot that my plan was to fire cancel anyway. I think we can add that on top of #10168 though.

[zcorpan]

Did #10168 fix this issue?

[domenic]

No, we still need to update the spec and tests to fire uncancelable cancel events.