Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WPT tests for unspecified PopoverInvokerElement removing steps #8991

Closed
nt1m opened this issue Mar 7, 2023 · 6 comments
Closed

WPT tests for unspecified PopoverInvokerElement removing steps #8991

nt1m opened this issue Mar 7, 2023 · 6 comments
Labels
topic: popover The popover attribute and friends

Comments

@nt1m
Copy link
Member

nt1m commented Mar 7, 2023

There is a WPT testing that moving/removing a PopoverInvokerElement hides its related popoverTargetElement when it was invoked from that button.

This doesn't look specified anywhere in the HTML spec. I don't have strong opinion on the behavior personally, but it would be nice to specify details like whether popoverTargetElement is still preserved after re-appending the element in the document.

cc @mfreed7 @josepharhar @annevk

@nt1m nt1m added the topic: popover The popover attribute and friends label Mar 7, 2023
@nt1m
Copy link
Member Author

nt1m commented Mar 9, 2023

This is the WPT in question: html/semantics/popovers/popover-target-element-disabled.html

Looks like adding the disabled attribute or changing the input type also counts as cutting the hierarchy, basically anything that would make popoverTargetElement inactive.

@nt1m
Copy link
Member Author

nt1m commented Mar 16, 2023

@rwlbuis @ziransun @asurkov Any opinion on this? There are 2 bits that WPT is testing don't seem to be in the spec:

  1. If any of the conditions for an element to be an invoker stops being met (attribute change), then the associated popover should be hidden.
  2. If an invoker is disconnected, the associated popover should be hidden

I think 2. is particularly problematic, since a web developer might just be moving the invoker and re-appending it somewhere else.

@annevk annevk added the agenda+ To be discussed at a triage meeting label Mar 16, 2023
@josepharhar
Copy link
Contributor

Apologies, here is a PR for the behavior which explains the purpose: #9048
If you're interested, additional context is in this code review where I added the test: https://chromium-review.googlesource.com/c/chromium/src/+/4115790

@nt1m nt1m removed the agenda+ To be discussed at a triage meeting label Mar 22, 2023
@asurkov
Copy link

asurkov commented Mar 23, 2023

@rwlbuis @ziransun @asurkov Any opinion on this? There are 2 bits that WPT is testing don't seem to be in the spec:

1. If any of the conditions for an element to be an invoker stops being met (attribute change), then the associated popover should be hidden.

2. If an invoker is disconnected, the associated popover should be hidden

I think 2. is particularly problematic, since a web developer might just be moving the invoker and re-appending it somewhere else.

It seems to me it's intuitive that if an invoker is disconnected then the associated popover is hidden. I'm not sure if re-appending the invoker button is a common scenario, but if it is, then I see how it can create a problem for the authors, because all they can do is to reimplement the popover invoker behaviour in JS, which is a bummer for web development for sure.

Also, I'm not sure exactly how the invoker disconnection is defined. From what I understand that a node removal is the case. But would display:none or @hidden, for example, be counted as well? What if a popover invoker is not on the screen (out of view or scrolled out)?

@josepharhar
Copy link
Contributor

But would display:none or hidden, for example, be counted as well? What if a popover invoker is not on the screen

I don't think that style and layout things like this should affect whether an invoker is considered valid. The current algorithms only look at DOM state like connectedness, position in the tree, and attributes.

@nt1m nt1m closed this as completed Jun 6, 2023
@nt1m
Copy link
Member Author

nt1m commented Jun 6, 2023

Closed as I think the "check and close popover stack" behavior is gone from the spec and WPT.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: popover The popover attribute and friends
Development

Successfully merging a pull request may close this issue.

4 participants