Should "show popover" assert the element's popover invoker is null?

[mbrodesser-Igalia]

At https://html.spec.whatwg.org/#show-popover before step 2. Or is it supposed to be replaceable?

[annevk]

I'm not sure how this can be true. There's nothing preventing another invocation of showPopover().

That does seem potentially not great as you can call it from somewhere else, change popover invoker, and then get an exception.

cc @nt1m @josepharhar

[mbrodesser-Igalia]

@annevk be aware that showPopover() (https://html.spec.whatwg.org/#dom-showpopover) invokes "show popover" with invoker = null.

[annevk]

Ah right, I think it can assert it then, looking at the callers.

[nt1m]

Seems like a reasonable assertion to add given "hide popover" sets the invoker to null.

[nt1m]

@josepharhar @mbrodesser-Igalia Would either of you be interested in making the spec PR?

[mbrodesser-Igalia]

@josepharhar @mbrodesser-Igalia Would either of you be interested in making the spec PR?

@nt1m yes, will do it.

[josepharhar]

I tried adding this assert in chromium, but it got hit in popover-invoker-reset.html.

In popover-invoker-reset, showPopover sets an invoker but then aborts showing the popover because the beforetoggle event handler prevents default, which then leaves us in the state where the popover has an invoker set but is closed. By calling showPopover again, the assert will be hit.

If we still want this assert then we need some way of clearing out the invoker at every exit point in showPopover or something.

[nt1m]

Should the invoker be asserted and manipulated after the "check popover validity" step?

[josepharhar]

Should the invoker be asserted and manipulated after the "check popover validity" step?

I think that wherever we put the assert doesn't matter. As long as it's possible to have the popover's invoker set to something while the popover is closed, I believe it will be possible to hit the assert

[mbrodesser-Igalia]

I tried adding this assert in chromium, but it got hit in popover-invoker-reset.html.

In popover-invoker-reset, showPopover sets an invoker but then aborts showing the popover because the beforetoggle event handler prevents default, which then leaves us in the state where the popover has an invoker set but is closed. By calling showPopover again, the assert will be hit.

@josepharhar: thanks for the explanation.

If we still want this assert then we need some way of clearing out the invoker at every exit point in showPopover or something.

Yes, that should work. Alternatively: postpone setting element's invoker to to invoker immediately after setting element's visibility state to showing and pass the invoker as an argument to https://html.spec.whatwg.org/#topmost-popover-ancestor and use it there.

That will be correct, because:

• https://html.spec.whatwg.org/#popover-invoker is at other places only accessed, when the visibility state is showing.
• https://html.spec.whatwg.org/#topmost-popover-ancestor is only called from https://html.spec.whatwg.org/#show-popover.

[mbrodesser-Igalia]

Anyone an opinion on the two proposals (#9383 (comment) and #9383 (comment))?

CC @annevk @nt1m

[josepharhar]

postpone setting element's invoker to to invoker immediately after setting element's visibility state to showing and pass the invoker as an argument to https://html.spec.whatwg.org/#topmost-popover-ancestor and use it there.

I prototyped this and it seems to work, so I am supportive. Lets do it!

What was the motivation for adding this assert in the first place though? Is it just that we shouldn't have an invoker set while the popover is closed because that state could be confusing?

Or is it supposed to be replaceable?

What did you mean by this?

[mbrodesser-Igalia]

postpone setting element's invoker to to invoker immediately after setting element's visibility state to showing and pass the invoker as an argument to https://html.spec.whatwg.org/#topmost-popover-ancestor and use it there.

I prototyped this and it seems to work, so I am supportive. Lets do it!

Okay, will create a spec-patch.

What was the motivation for adding this assert in the first place though? Is it just that we shouldn't have an invoker set while the popover is closed because that state could be confusing?

Yes, exactly.

Or is it supposed to be replaceable?

What did you mean by this?

If the invoker element was supposed to stay (that is, non-null) and then be replaced by another non-null element.