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 subtest testing beforetoggle during showPopover() #9037

Closed
rwlbuis opened this issue Mar 16, 2023 · 3 comments · Fixed by #9058
Closed

WPT subtest testing beforetoggle during showPopover() #9037

rwlbuis opened this issue Mar 16, 2023 · 3 comments · Fixed by #9058
Labels
topic: popover The popover attribute and friends

Comments

@rwlbuis
Copy link

rwlbuis commented Mar 16, 2023

In popover-attribute-basic.html there is a subtest to test beforetoggle behaviour during showPopover(). There is an expectation that an exception is thrown because the beforetoggle event handler changes the popover type. I believe this is dictated by:

  1. If originalType is not equal to the value of element's [popover] attribute, or if the result of running [check popover validity] given element, false, and throwExceptions is false, then return.

[check popover validity] can throw an exception but AFAICS it should not in this case. The first condition does apply (originalType != new type) but the way I read the spec it will not throw an exception but just return.

If above is correct, it seems specification and test do not seem to match. Thought?

@rwlbuis
Copy link
Author

rwlbuis commented Mar 16, 2023

cc @annevk @nt1m @mfreed7 @josepharhar

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

annevk commented Mar 17, 2023

This regressed in 031d3f1.

Simplest fix would be to break up this conditional and add the relevant throwing or return language back for the first part.

josepharhar added a commit to josepharhar/html that referenced this issue Mar 21, 2023
Since exception throwing was moved into "check popover validity," we
need to explicitly throw an exception for this originalType check in
order to make sure that it actually throws an exception like it is
supposed to.

Fixes whatwg#9037
@josepharhar
Copy link
Contributor

I created a PR to fix this: #9058

annevk pushed a commit that referenced this issue May 26, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Since exception throwing was moved into "check popover validity," we need to explicitly throw an exception for this originalType check in order to make sure that it actually throws an exception like it is supposed to.

Fixes #9037.
@whatwg whatwg deleted a comment from ah666677 May 29, 2023
rubberyuzu pushed a commit to rubberyuzu/html that referenced this issue Jul 21, 2023
This fixes a typo in the definition of "fully active descendant of a top-level traversable with user attention": s/traversible/traversable.

Popover: throw exception in originalType check

Since exception throwing was moved into "check popover validity," we need to explicitly throw an exception for this originalType check in order to make sure that it actually throws an exception like it is supposed to.

Fixes whatwg#9037.

Rename intrinsic dimensions to natural dimensions to match CSS

Also fix some instances to use the "density-corrected" variant instead.

Fixes whatwg#6233.
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