"check popover algorithm" check for open dialogs is weak

[nt1m]

https://html.spec.whatwg.org/multipage/popover.html#check-popover-validity

element is a dialog element and has an open attribute

This check doesn't cover this case (in terms of top layer conflicts):

<script>
  onload = () => {
    dialog0.showModal();
    dialog0.open = false;
    dialog0.showPopover();
  };
</script>
<dialog id="dialog0" popover=""></dialog>

Should we change this check to checking whether the dialog is in modal state instead?

cc @annevk @domenic @mfreed7 @josepharhar @emilio @cathiechen

[mfreed7]

Hmm, the behavior in Chromium is that dialog0 is left showing as a popover. And that's what I'd expect, I guess, right? Removing the open attribute on the modal dialog closes it, and it's free to be re-opened as a popover.

(This is precisely why I really didn't want to add an open attribute for popovers.)

[nt1m]

Removing the open attribute on the modal dialog closes it

This isn't the case per the spec right now, the modal dialog will be hidden (display: none), but still in the top layer.

(WebKit caught this because this triggered an assertion)

[mfreed7]

This isn't the case per the spec right now, the modal dialog will be hidden (display: none), but still in the top layer.

Boo. That's the case also in Chromium, but it is rendered display:none. However, the popover behavior does seem to still work correctly I think. There could be some issues behind the scenes.

I wonder if we shouldn't change the spec so that removing open closes the dialog and explicitly removes from the top layer?

[nt1m]

I wonder if we shouldn't change the spec so that removing open closes the dialog and explicitly removes from the top layer?

I think this might be an entirely separate discussion, because we'd want to figure out what .open = true does.

However, the popover behavior does seem to still work correctly I think. There could be some issues behind the scenes.

I think we do want to avoid top layer conflicts (e.g. having something that is opened in top layer for 2 different reasons), because it can leave the element in the broken state once you get to the point of removing it from the top layer. Also, it makes the stacking order unclear in the top layer if there are multiple elements.

I'd personally be in favor in changing the check from checking for the "open attribute" to checking for the "modal state", since I think the original intent for the check is to prevent top layer conflicts.

[mfreed7]

I wonder if we shouldn't change the spec so that removing open closes the dialog and explicitly removes from the top layer?

I think this might be an entirely separate discussion, because we'd want to figure out what .open = true does.

Yeah, I agree. Again, I wish we could get rid of open.

However, the popover behavior does seem to still work correctly I think. There could be some issues behind the scenes.

I think we do want to avoid top layer conflicts (e.g. having something that is opened in top layer for 2 different reasons), because it can leave the element in the broken state once you get to the point of removing it from the top layer. Also, it makes the stacking order unclear in the top layer if there are multiple elements.

I'd personally be in favor in changing the check from checking for the "open attribute" to checking for the "modal state", since I think the original intent for the check is to prevent top layer conflicts.

Given the above, I agree that it's probably good to change the check from [open] to "is modal". I'm supportive.

[domenic]

That separate discussion is #5802, and would be great if people were willing to tackle it.

[nt1m]

I created #9344

[nt1m]

@mfreed7 FWIW, a side-effect of my PR is that we can call showPopover() on top of show() or any non-modal dialog now. I think it's not particularly troublesome, but if you think it is, I can change the check from "is modal" to "is open or modal".