`HTMLElement.togglePopover` can't close the popover when applied on `<td>`

[LcsGa]

What is the issue with the HTML Standard?

I've been facing this weird issue where the togglePopover doesn't work when it is for closing the popover on a <td> element (I don't know if it's the only element that has this issue though).

My popover is set to auto (by default) and I toggle it with that method (which always wroked well until then, but I may be missing something) (never faced any issue with popovertarget, that's what I mean).

After doing my best to debug it, I've found that the beforetoggle event is triggered twice (close then open) (but not the toggle one, strangely). => if I prevent the opening one (when concurrent with the closing one), then everything works find (this is my only workaround for now).

Here is a reproduction of the issue (and warkaround) https://codepen.io/lcsga/pen/wBwPeMY

If I forget to provide some other needed informations, let me know, I'd do my best to complete it asap!

[scottaohara]

i looked at your codepen and noticed you commented out the workaround. did you end up resolving your issue? i'm not having issues opening or closing the popovers in your test using mouse or keyboard? maybe i'm missing something.

but with that said - i'm not sure i really understand the use case here. why you wouldn't put a button within those TDs to mitigate the issue you're potentially facing / make this accessible for people using screen readers. TD elements (role=cell) don't support the expanded state in the a11y tree, so there'd be a lot of extra work needed to ensure this behavior would be properly conveyed to people using screen readers. That is, unless you nested button elements inside the TDs and used those to toggle the popovers - then no JS or extra ARIA for your buttons would be necessary.

[LcsGa]

No I didn't find any other solution yet, I just commented it so that people testing it could see the issue right away.
Don't you have the issue when you click on the click me cell only? If I do, I still have the same behavior => would open but won't close (If I click anywhere else then it works fine though and didn't mention it, my bad!)

About your second point, you're completely right! My answer might not convince you (and for a good reason, because I simply avoid another problem) but I did it because the popover would not align correctly with mat-tables when the target is something within a cell, and not the cell itself...
I unfortunately have to use this library (that I must not override as much as possible) for tables (and everything else btw) (I don't have that alignement issue with "vanilla" tables).
I know the solution I should find should be related to this alignement issue instead, but still, the behavior of togglePopover is still weird there, isn't it?

[annevk]

I'm not immediately sure what's wrong here, but this also reproduces if I replace the <td>s with <div>s.

[LcsGa]

Nice to know! It might not be as niche as I first thought then!

By the way, I made this angular material reproduction of my original alignement issue that made me try using the cell itself as the target, if you're intersted into it too (but it might need its own issue and might be "out of context" here)

[annevk]

I can also reproduce with

<div id=popover1 popover>popover</div>
<div onclick=popover1.togglePopover()>test</div>

but not with popover=manual. Maybe @nt1m or @josepharhar will know what's up. It does seem that if this is the intended behavior we should update the web-developer-facing documentation for togglePopover().

[LcsGa]

By the way, reading you helped me rethink about it and I found a way better workaround for my use case:

• keeping the button as the popovertarget
• anchor-name on the td

=> no alignement issue anymore, no bug, no a11y issue!

[josepharhar]

I can reproduce as well. I think what's happening here is that light dismiss is closing the popover when clicking on the invoking button before the button's click handler toggles the popover, which means that the popover is getting synchronously closed and then opened again.

A workaround for now is to just use showPicker instead of togglePicker, which should give you the desired behavior.

This is able to toggle the popover without having the bad behavior of closing and re-opening, and I forget what we added to make it work:

<div popover=auto id=popover>popover</div>
<button popovertarget=popover popovertargetaction=toggle>toggle</button>

[oddnugget]

Having the same issue, and i agree with @josepharhar on what i think is happening.

A workaround is to track the state manually:

    let open = false;

    const content = document.getElementById("myPopover");
    const button = document.getElementById("myButton");

    content.addEventListener("toggle", (e) => (open = e.newState == "open"));

    button.addEventListener("click", () => {
      if (open) {
        content.hidePopover();
      } else {
        content.showPopover();
      }
    });

[bakura10]

I'm facing the same issue. togglePopover has a weird behavior and is really counter-intuitive. I was expecting calling togglePopover to work in the same way as using the declarative approach. @oddnugget workaround works but it feels quite hacky :/

[josepharhar]

I just tested this out again with my proposed light dismiss changes implemented, and unfortunately it didn't fix the bug.

[mfreed7]

So I think this is actually a bug in the algorithms. The "right" way to do this should be this:

<div id=popover1 popover>popover</div>
<div onclick=popover1.togglePopover({source: this})>test</div>

I.e. the source parameter should connect the trigger to the popover, which should then get used during the light dismiss algorithm. Currently, light dismiss uses the "topmost clicked popover" algorithm, which calls "nearest inclusive target popover", and that uses "popover target element" to find invokers to include in the light dismiss algorithm. Unfortunately, that last algorithm just checks for popovertarget. Interestingly it doesn't even look for commandfor combined with command={something}-popover.

Checking this repro (https://jsbin.com/bohagab/edit?html,output), it seems that both Chromium and Gecko DO properly look at the command invokers and do the right thing. (WebKit doesn't implement commandfor yet.) None properly handle the imperatively-added source from the togglePopover() call. It feels like "popover target element" should be updated to include both commandfor popover invokers (to match browser behavior) and also include the "reverse" concept of the popover trigger. I say "reverse", because we'd need to store a pointer on the invoking/source element pointing to the popover. That might get tricky - it might be possible to have one invoker pointing to more than one open popover.

In the meantime, the currently-supported way to do this is to use either of the declarative ways to invoke popovers:

<div id=popover2 popover>popover</div>
<button popovertarget=popover2>test 2</button>

<div id=popover3 popover>popover</div>
<button commandfor=popover3 command="toggle-popover">test 3</button>