1874476 - [wpt-sync] Sync PR 43976 - Fix popover crash bug

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, task, P4)

Description

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Sync web-platform-tests PR 43976 into mozilla-central (this bug is closed when the sync is complete).

PR: https://github.com/web-platform-tests/wpt/pull/43976
Details from upstream follow.

Mason Freed <[e-mail]> wrote:

Fix popover crash bug

It was previously possible to hit a CHECK (and I think some
underspecified behavior) with something like this:

\<div popover id=p1>Popover 1
\<div popover id=p2>Popover 2\</div>
\</div>
\<script>
p1.showPopover();
p1.addEventListener('beforetoggle',() => p2.showPopover());
p1.hidePopover();
\</script>

The problem is that "hide all popovers until" doesn't end up with
the desired "until" popover on the top of the stack in this case.
There is already a similar situation within the "hide all..."
algorithm itself, but that only handles the case where a popover
being hidden by "hide all..." has the beforetoggle listener. This
is the same problem, but for the case that the "until" popover
has that listener.

I believe this needs a spec update. WebKit doesn't crash (likely
it doesn't have the same CHECKs that Blink has), but also doesn't
result in both p1 and p2 being hidden in the above example.

Fixed: 1513282
Change-Id: I463888096db9a8983d355f246991bbbad596d1f2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5141430
Auto-Submit: Mason Freed \<[e-mail]>
Commit-Queue: Mason Freed \<[e-mail]>
Reviewed-by: Joey Arhar \<[e-mail]>
Cr-Commit-Position: refs/heads/main@{#1246585}

Comment 1

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

PR 43976 applied with additional changes from upstream: 51c87dc4c5d4a61caef22344e4dba6f5f233ffc3

Comment 2

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Pushed to try (stability) https://treeherder.mozilla.org/#/jobs?repo=try&revision=d2cf38b80d01832b3e1c9d9b4c330e5de6c95110

Comment 3

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

CI Results

Ran 0 Firefox configurations based on mozilla-central, and Firefox, Chrome, and Safari on GitHub CI

Total 1 tests and 4 subtests

Status Summary

Firefox

OK : 1
PASS : 2
FAIL : 2

Chrome

CRASH: 1

Safari

OK : 1
PASS : 2
FAIL : 2

Links

GitHub PR Head
GitHub PR Base

Details

New Tests That Don't Pass

• /html/semantics/popovers/popover-open-in-beforetoggle.html [wpt.fyi]
  • Open popover from closing beforetoggle event: FAIL (Safari: FAIL)
  • Open double-nested popovers from closing beforetoggle event: FAIL (Safari: FAIL)

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/89df0631328f
[wpt PR 43976] - Fix popover crash bug, a=testonly
https://hg.mozilla.org/integration/autoland/rev/f223066728b8
[wpt PR 43976] - Update wpt metadata, a=testonly

Comment 5

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/89df0631328f
https://hg.mozilla.org/mozilla-central/rev/f223066728b8