When the popover attribute is removed, the hide popover algorithm will return too early

[mbrodesser-Igalia]

Given whatwg/dom#1176, the popover attribute is removed before running the element's attribute change steps.
Those steps are defined for the popover attribute: https://html.spec.whatwg.org/#attr-popover which in step 3 invoke the hide popover algorithm (https://html.spec.whatwg.org/#hide-popover-algorithm). That invokes checking the popover validity (https://html.spec.whatwg.org/#check-popover-validity) as its first step and if it returns false, returns early. The latter function checks in its first step, whether the element's popover attribute is in the "no popover state". Hence,

somePopoverElement.showPopover();
setTimeout(() => {
    somePopoverElement.removeAttribute("popover");
}, 1000);

will return early from the hide popover algorithm. Hence, the popover will remain open.

Seems related to #9161 and whatwg/dom#1185 (comment)

CC @josepharhar @jakearchibald @rwlbuis

[mbrodesser-Igalia]

Since Chromium's implementation seems to have been the main inspiration for the spec, it seems worth to mention that implementation is sometimes imprecisely checking for whether an element has has the popover attribute: https://source.chromium.org/chromium/chromium/src/+/refs/heads/main:third_party/blink/renderer/core/html/html_element.cc;l=1277;drc=c4efcabd32a851d4d7816206d3c8793a55e9d57e

So one way forward could be to check for which callers that method's return value might differ from the element actually having a popover attribute.

[josepharhar]

I see. My first thought would be to add a parameter to hide popover and to check popover validity called "skipAttributeCheck", which would be set to true when hide popover is called from the attribute removal steps.

So one way forward could be to check for which callers that method's return value might differ from the element actually having a popover attribute.

I don't fully understand... is this the same as my "skipAttributeCheck" idea?

[mbrodesser-Igalia]

I see. My first thought would be to add a parameter to hide popover and to check popover validity called "skipAttributeCheck", which would be set to true when hide popover is called from the attribute removal steps.

That seems rather hacky. That parameter would have to be passed to all three calls of "check popover validity" in the hide popover algorithm. The thing is, it opens the door for misuse of of the hide popover algorithm and "check popover validity".

So one way forward could be to check for which callers that method's return value might differ from the element actually having a popover attribute.

I don't fully understand... is this the same as my "skipAttributeCheck" idea?

No. It's an approach which might help to determine a cleaner solution for this issue.

[mbrodesser-Igalia]

To be explicit, the "skipAttributeCheck" idea should work.

[josepharhar]

So one way forward could be to check for which callers that method's return value might differ from the element actually having a popover attribute.

Is "that method" the "check popover validity" algorithm? I don't see how we can get around this without changing check popover validity to not look at the attribute or replacing the call to check popover validity with something else that doesn't care about the attribute.

[mbrodesser-Igalia]

So one way forward could be to check for which callers that method's return value might differ from the element actually having a popover attribute.

Is "that method" the "check popover validity" algorithm? I don't see how we can get around this without changing check popover validity to not look at the attribute or replacing the call to check popover validity with something else that doesn't care about the attribute.

Sorry for the imprecision. With "that method", HTMLElement::HasPopoverAttribute (https://source.chromium.org/chromium/chromium/src/+/refs/heads/main:third_party/blink/renderer/core/html/html_element.cc;l=1277;drc=c4efcabd32a851d4d7816206d3c8793a55e9d57e) was meant.
After more analysis of the spec, I currently also don't see another solution than adding "skipAttributeCheck".

[josepharhar]

Maybe "skipAttributeCheck" could be expanded to a "ignoreDomState" parameter which would also satisfy the need to hide the popover after it has been disconnected: whatwg/dom#1185 (comment)

Instead of modifying the check popover algorithm, we could just replace calls to the check popover algorithm with a simple check of whether or not the popover is in the showing state when the hide popover algorithm is called with the "ignoreDomState" option

[josepharhar]

I opened a PR to implement my last comment: #9457