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.

Sign up for GitHub

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

Jump to bottom

nestedShow is not initialized in https://html.spec.whatwg.org/#show-popover #9429

Closed
mbrodesser-Igalia opened this issue Jun 15, 2023 · 2 comments · Fixed by #9439
Closed

nestedShow is not initialized in https://html.spec.whatwg.org/#show-popover #9429

mbrodesser-Igalia opened this issue Jun 15, 2023 · 2 comments · Fixed by #9439
Labels
topic: popover The popover attribute and friends

Comments

@mbrodesser-Igalia
Copy link
Member

That seems trivial to fix. However, before doing that, can someone please provide an example, in which the nested show mechanism reduces the length of the call stack (#9198 (comment))?
@mbrodesser-Igalia mbrodesser-Igalia added the topic: popover The popover attribute and friends label Jun 15, 2023
@annevk
Copy link
Member

annevk commented Jun 15, 2023

The problem seems to be that it's initialized too late, right?

Maybe @cathiechen can help out with an example.

@cathiechen
Copy link

cathiechen commented Jun 15, 2023 •
edited
Loading

Sure:)

<!doctype html>
<div id=p16 popover="auto">
    <div id=p17 popover="auto">17</div>
    <div id=p18 popover="auto">18</div>
    <div id=p19 popover="auto">19</div>
</div>
<script>
    let events = [];
    const logEvents = (e) => { events.push(`${e.newState === 'open' ? 'show' : 'hide'} ${e.target.id}`) };

    p16.showPopover();
    p17.showPopover();
    p17.addEventListener('beforetoggle', (e) => {
        logEvents(e);
        p19.showPopover();
    });
    p18.addEventListener('beforetoggle', (e) => {
        logEvents(e);
    });
    p19.addEventListener('beforetoggle', (e) => {
        logEvents(e);
        p18.showPopover();
    });
    p18.showPopover();
    console.log(events)
</script>

And I will upstream the case to WPT along with the Gecko patch. https://phabricator.services.mozilla.com/D181121

mbrodesser-Igalia added a commit to mbrodesser-Igalia/html that referenced this issue Jun 19, 2023
@mbrodesser-Igalia
Change "show popover" to call "check popover validity" first
25edd1c 
This is what implementations (at least Gecko and Chromium) do and it's analogous to
what happens in "hide popover".

Fixes whatwg#9429.
@mbrodesser-Igalia mbrodesser-Igalia mentioned this issue Jun 19, 2023
Merged
annevk pushed a commit that referenced this issue Jun 29, 2023
@mbrodesser-Igalia
Editorial: change "show popover" to call "check popover validity" first
66e162f 
This is what implementations (at least Gecko and Chromium) do and it's analogous to what happens in "hide popover".

Fixes #9429.
rubberyuzu pushed a commit to rubberyuzu/html that referenced this issue Jul 20, 2023
@mbrodesser-Igalia
Editorial: change "show popover" to call "check popover validity" first
4a377a7 
This is what implementations (at least Gecko and Chromium) do and it's analogous to what happens in "hide popover".

Fixes whatwg#9429.
rubberyuzu pushed a commit to rubberyuzu/html that referenced this issue Jul 21, 2023
@mbrodesser-Igalia
Editorial: change "show popover" to call "check popover validity" first
3a0f060 
This is what implementations (at least Gecko and Chromium) do and it's analogous to what happens in "hide popover".

Fixes whatwg#9429.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
topic: popover The popover attribute and friends
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Editorial: change "show popover" to call "check popover validity" first mbrodesser-Igalia/html
3 participants
@annevk @cathiechen @mbrodesser-Igalia