html icon indicating copy to clipboard operation
html copied to clipboard

Consider throwing for showModal() and showPopover() in non-fully-active documents

Open domenic opened this issue 1 year ago • 12 comments

What is the issue with the HTML Standard?

As a followup to #10634, it doesn't really make sense that you can call showModal() or showPopover() on elements which are inside of synthetic documents, of the type created by document.implementation.createHTMLDocument(). Example: https://jsbin.com/cubafoputo/1/edit?html,js,output

I think we should have these throw exceptions, similar to the exceptions we already throw for disconnected elements.

I suspect this change is web-compatible, especially for popover but even for dialog. These synthetic documents are super rare.

@smaug---- @keithamus @josepharhar @nt1m do you agree? Are you willing to take the potential compat risk or the trouble of adding use counters?

domenic avatar Sep 30 '24 04:09 domenic

Sounds reasonable to me (non-implementer, userland consumer). They already throw for a handful of exceptional cases (calling showModal() twice for example) which are much more likely to occur.

keithamus avatar Sep 30 '24 08:09 keithamus

Given how new popover is, how most uses of popover are probably based on user input, and how niche this type of document is, my instinct is also that this would be web compatible. Likewise for dialogs, they're relatively new too.

Should this apply to dialog.show() too?

lukewarlow avatar Sep 30 '24 11:09 lukewarlow

I'm unsure about show(). If we were to ever fix #5802 and make show() roughly equivalent to adding open="", then it would not make much sense to throw, I think.

domenic avatar Oct 01 '24 01:10 domenic

Throwing sounds good to me.

smaug---- avatar Oct 01 '24 09:10 smaug----

Is the idea here that nothing should be in the top layer when a document is no longer fully active? Will we also empty it when a document becomes inactive?

annevk avatar Oct 01 '24 11:10 annevk

Is the idea here that nothing should be in the top layer when a document is no longer fully active? Will we also empty it when a document becomes inactive?

No, I think the idea is that nothing should be in the top layer for synthetic documents. "Normal" documents that get bfcached can have a top layer without an issue, IMO.

domenic avatar Oct 02 '24 01:10 domenic

Wouldn't a browsing context check be more appropriate then? I guess either way could work, but the former is a more exact match of what you're after.

annevk avatar Oct 02 '24 05:10 annevk

Indeed, they're equivalent in this case. Fully active is just more conventional.

domenic avatar Oct 02 '24 05:10 domenic

Well, doesn't a bfcache'd document still hold onto its browsing context?

annevk avatar Oct 02 '24 06:10 annevk

Yes, but you cannot call showModal() on a <dialog> in a bfcached document, because you cannot have a scripting relationship to such documents.

domenic avatar Oct 02 '24 06:10 domenic

This sounds reasonable, and likely to be web compatible, at least to me. It seems very corner-case to create a document, put a popover in it, and call showPopover() expecting things to happen. There's always the chance that there's a use case, but I can't think of one. I suppose it's possible that people do that and then move those elements to a rendered document and expect things to be "still open", but that's not what happens today, other than an open non-modal dialog because of the open attribute.

I'd be relatively happy to take the risk of compat shipping this change, if there's consensus.

mfreed7 avatar Oct 09 '24 16:10 mfreed7

Thanks all. I think that's explicit support from 2/3 implementations and some level of implicit agreement from @annevk, so I'll take this off of agenda+ and proceed with a spec PR asynchronously.

domenic avatar Oct 10 '24 01:10 domenic