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

Add "hide all popovers" algorithm #8886

Merged
merged 6 commits into from
Feb 23, 2023
Merged

Conversation

josepharhar
Copy link
Contributor

@josepharhar josepharhar commented Feb 14, 2023

This was requested for cross referencing instead of "hide all popovers until" here:
whatwg/fullscreen#204 (comment)

This patch also fixes a bug where a null endpoint is passed to hide all popovers until, but hide all popovers until assumes endpoint is not null by looking for endpoint's node document. The bug is fixed by adding a required document parameter to hide all popovers until.

  • At least two implementers are interested (and none opposed):
  • Tests are written and can be reviewed and commented upon at:
  • Implementation bugs are filed:
    • Chromium: …
    • Gecko: …
    • WebKit: …
    • Deno (only for timers, structured clone, base64 utils, channel messaging, module resolution, web workers, and web storage): …
    • Node.js (only for timers, structured clone, base64 utils, channel messaging, and module resolution): …
  • MDN issue is filed: …

(See WHATWG Working Mode: Changes for more details.)


/interactive-elements.html ( diff )
/popover.html ( diff )

This was requested for cross referencing instead of "hide all popovers
until" here:
whatwg/fullscreen#204 (comment)

This patch also fixes a bug where a null endpoint is passed to hide all
popovers until, but hide all popovers until assumes endpoint is not null
by looking for endpoint's node document. The bug is fixed by adding a
required document parameter to hide all popovers until.
@annevk
Copy link
Member

annevk commented Feb 15, 2023

I wish I had seen this before creating #8889. Oh well.

source Outdated Show resolved Hide resolved
@annevk
Copy link
Member

annevk commented Feb 15, 2023

As part of this PR, please also double check that all the relevant terms and algorithms have been exported so whatwg/fullscreen#204 can reference them. There are some CI errors that indicate this hasn't happened yet.

@josepharhar
Copy link
Contributor Author

As part of this PR, please also double check that all the relevant terms and algorithms have been exported so whatwg/fullscreen#204 can reference them. There are some CI errors that indicate this hasn't happened yet.

I made the new algorithm have <dfn export> so I figured that is good enough. The CI errors you're referring to are in the fullscreen PR, right? I expect there to be errors until this PR is merged and the algorithm is exported

@annevk annevk added the topic: popover The popover attribute and friends label Feb 16, 2023
@josepharhar josepharhar mentioned this pull request Feb 17, 2023
4 tasks
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If endpoint is document, do we need to set endpoint to null?

source Show resolved Hide resolved
source Show resolved Hide resolved
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clarification, one nit left.

source Outdated Show resolved Hide resolved
@annevk annevk merged commit 6bb5df0 into whatwg:main Feb 23, 2023
josepharhar added a commit to josepharhar/html that referenced this pull request Mar 1, 2023
In whatwg#8886 I made the endpoint
parameter in hide all popovers until never null by making it either a
document or an element, but I forgot to change this null check to a
document check.

Fixes whatwg#8963
annevk pushed a commit that referenced this pull request Mar 1, 2023
In #8886 I made the endpoint parameter in hide all popovers until never null by making it either a document or an element, but I forgot to change this null check to a document check.

Fixes #8963.
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 this pull request may close these issues.

2 participants