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

Should step 7 of https://html.spec.whatwg.org/#hide-popover-algorithm be a while loop? #9468

Closed
mbrodesser-Igalia opened this issue Jun 29, 2023 · 5 comments · Fixed by #9528
Closed

Should step 7 of https://html.spec.whatwg.org/#hide-popover-algorithm be a while loop? #9468

mbrodesser-Igalia opened this issue Jun 29, 2023 · 5 comments · Fixed by #9528
Labels
topic: popover The popover attribute and friends

Comments

@mbrodesser-Igalia
Copy link
Member

Currently it's an if condition, followed by a break in step 7.3.

@cathiechen @josepharhar
@mbrodesser-Igalia mbrodesser-Igalia added the topic: popover The popover attribute and friends label Jun 29, 2023
@cathiechen
Copy link

"If element's popover attribute is in the auto state, then:"
Looks like we could use "While" instead of "If" to reduce the confusion?

@cathiechen
Copy link

Sorry, I was wrong.

Looks like 6.6 of hide popovers until could make sure element is the last element of auto popover list. So there is no need to repeat in hidepopover.

Maybe we could remove this? "If the last item in document's auto popover list is element, then break."

@josepharhar
Copy link
Contributor

Oh weird, this definitely isn't right.
Based on the chromium implementation, there is no such auto popover list check or break.

It looks like I changed this from an assert to a check in this PR: #9198
I need to do some more archeology to figure out if I messed this up or if that check was later removed in chromium.

@josepharhar
Copy link
Contributor

Ok, I think the reason that I mistakenly added that break is because I was trying to get this chromium patch into the spec and I was going to make it a while(true) loop or something - the condition of the break matches the condition of the while in that chromium patch.

I'll make a spec PR shortly to get this fixed

josepharhar added a commit to josepharhar/html that referenced this issue Jul 17, 2023
@josepharhar
Remove erroneous break in hide popover algorithm
f679518 
Fixes whatwg#9468

I mistakenly added and modified this check while speccing a chromium
patch in this PR: whatwg#9198
@josepharhar josepharhar mentioned this issue Jul 17, 2023
Merged
3 tasks
@josepharhar
Copy link
Contributor

Spec PR: #9528

annevk pushed a commit that referenced this issue Jul 25, 2023
@josepharhar
Remove erroneous break in hide popover algorithm
eed5c0e 
I mistakenly added and modified this check in #9198.

Fixes #9468.
rubberyuzu pushed a commit to rubberyuzu/html that referenced this issue Aug 25, 2023
@josepharhar
Remove erroneous break in hide popover algorithm
0f0cc2c 
I mistakenly added and modified this check in whatwg#9198.

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

Remove erroneous break in hide popover algorithm josepharhar/html
3 participants
@josepharhar @cathiechen @mbrodesser-Igalia