1821732 - implement popover light dismiss

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement)

Description

[alexander :surkov (:asurkov)]

here's relevant spec part https://html.spec.whatwg.org/multipage/popover.html#popover-light-dismiss

Comment 1

[alexander :surkov (:asurkov)]

Attachment: Bug 1821732 - implement auto popover list, r?emilio

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dee2dc3c9b83
implement auto popover list, r=emilio

Comment 3

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/dee2dc3c9b83

Comment 4

[alexander :surkov (:asurkov)]

more patches are coming

Comment 5

[alexander :surkov (:asurkov)]

Attachment: Bug 1821732 - implement "hide all popovers unitl" algorithm, r?emilio

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/34b104a10e48
implement "hide all popovers unitl" algorithm, r=emilio

Comment 7

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/34b104a10e48

Comment 8

[alexander :surkov (:asurkov)]

Attachment: Bug 1821732 - implement the nearest inclusive open popover algorithm, r?emilio

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2622ca0f5a6c
implement the nearest inclusive open popover algorithm, r=emilio

Comment 10

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/2622ca0f5a6c

Comment 11

[alexander :surkov (:asurkov)]

Attachment: WIP: Bug 1821732 - support PopoverInvokerElement

Comment 12

[Phabricator Automation]

Comment on attachment 9324239 [details]
WIP: Bug 1821732 - support PopoverInvokerElement

Revision D173180 was moved to bug 1823757. Setting attachment 9324239 [details] to obsolete.

Comment 13

[alexander :surkov (:asurkov)]

Attachment: Bug 1821732 - implement nearest invlusive target popover for invoker alg, r?emilio

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/41dd97c2aeaf
implement nearest invlusive target popover for invoker alg, r=emilio

Comment 15

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/41dd97c2aeaf

Comment 16

[alexander :surkov (:asurkov)]

Attachment: Bug 1821732 - implement the topmost clicked popover algorithm, r?emilio

See https://html.spec.whatwg.org/multipage/popover.html#topmost-clicked-popover spec.
In support of popover light dismiss, see
https://html.spec.whatwg.org/multipage/popover.html#popover-light-dismiss

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a3a7ad63c14a
implement the topmost clicked popover algorithm, r=emilio

Comment 18

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/a3a7ad63c14a

Comment 19

[alexander :surkov (:asurkov)]

Attachment: Bug 1821732 - implement get popover target alg, r?emilio

See https://html.spec.whatwg.org/multipage/popover.html#popover-target-element

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c82f0271d0f6
implement get popover target alg, r=emilio

Comment 21

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/c82f0271d0f6

Comment 22

[alexander :surkov (:asurkov)]

Attachment: Bug 1821732 - implement light dismiss open popovers on pointer events, r?emilio

See https://html.spec.whatwg.org/multipage/popover.html#popover-light-dismiss

Comment 23

[alexander :surkov (:asurkov)]

Attachment: Bug 1821732 - polyfill the declarative shadow DOM feature in popover-light-dismiss.html test, r?emilio

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bdf80a0626c0
polyfill the declarative shadow DOM feature in popover-light-dismiss.html test, r=emilio

Comment 25

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

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/39910 for changes under testing/web-platform/tests

Comment 26

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/bdf80a0626c0

Comment 27

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

Upstream PR merged by moz-wptsync-bot

Comment 28

[alexander :surkov (:asurkov)]

Attachment: Bug 1821732 - GetTopmostClickedPopover fails to return a popover, r?emilio

Temporary auto popover list variable is freed while it's used for mozilla::Reversed, thus
GetTopmostClickedPopover comparisment fails.

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4b1185a310de
GetTopmostClickedPopover fails to return a popover, r=emilio

Comment 30

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/4b1185a310de

Comment 31

[alexander :surkov (:asurkov)]

Attachment: Bug 1821732 - popover-light-dismiss tests shouldn't reuse existing actions, r?emilio

Actions don't drop their state on send, subsequent send() call retriggers the whole sequence
which may lead to unexpected errors, see https://github.com/web-platform-tests/wpt/issues/40062

Comment 32

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e6916184642e
popover-light-dismiss tests shouldn't reuse existing actions, r=emilio

Comment 33

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

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/40179 for changes under testing/web-platform/tests

Comment 34

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/e6916184642e

Comment 35

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

Upstream PR merged by moz-wptsync-bot

Comment 36

[alexander :surkov (:asurkov)]

Attachment: Bug 1821732 - fix 'Popovers close on pointerup, not pointerdown' test in popover_light_dismiss.html, r?emilio

This is a followup fix for https://phabricator.services.mozilla.com/D178535

Comment 37

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0f52c6760d33
fix 'Popovers close on pointerup, not pointerdown' test in popover_light_dismiss.html, r=emilio

Comment 38

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

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/40200 for changes under testing/web-platform/tests

Comment 39

[alexander :surkov (:asurkov)]

Attachment: Bug 1821732 - get browser context should not fail for a non Element, r?jgraham

Internal wpt webdriver error may cause get_context call for an object in the case,
for example, of subsequent sending of the webdriver actions, which alters action origin
from an element to an object.

Comment 40

[alexander :surkov (:asurkov)]

Attachment: Bug 1821732 - throw an error if webdriver action fails, r?jgraham

Promise reject should always go with an Error object because unhandled
promise throws the given value.

If webdriver action fails, then it prints stack in terminal and all the wpt testsuite
has is {status:error} object with no context. The given data has no |message|
field either so the promise is rejected with 'error: undefined' string which makes
a very misterious error. Throwing a real error with a good message gives a stack
and a hint how to debug the failure.

Comment 41

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/0f52c6760d33

Comment 42

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

Upstream PR merged by moz-wptsync-bot

Comment 43

[alexander :surkov (:asurkov)]

Attachment: Bug 1821732 - popover-light-dismiss test should have no dependency on anchor

HTML anchor is a separate spec from popover spec and they should be tested
independently. Having unitary testing for different spec helps to uncover bugs
easily as well as it helpful to test one spec while the other one is not yet
implemented. Now popover-light-dismiss test is neary useless if anchor spec is
not implemented, because the test just fails.

If integration tests to cover popover and anchor specs are needed, they should
be added separately.

Comment 44

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/63f48f5eacea
popover-light-dismiss test should have no dependency on anchor r=mbrodesser-Igalia,emilio

Comment 45

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

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/41189 for changes under testing/web-platform/tests

Comment 46

[Norisz Fay [:noriszfay]]

Backed out for causing failures on popover-light-dismiss-with-anchor.tentative.html

Backout link

Push with failures

Failure log

Comment 47

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

Upstream PR was closed without merging

Comment 48

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/80431152bbf5
popover-light-dismiss test should have no dependency on anchor r=mbrodesser-Igalia,emilio

Comment 49

[Natalia Csoregi [:nataliaCs]]

Backed out for causing failures on popover-light-dismiss.html

• backout: https://hg.mozilla.org/integration/autoland/rev/2948f67bdb67c3681daf7b86428ddb74f2612477
• push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&revision=80431152bbf51f98db6263984c84c766bb464440
• failure log: https://treeherder.mozilla.org/logviewer?job_id=424075023&repo=autoland&lineNumber=4407

[task 2023-07-27T20:27:07.975Z] 20:27:07     INFO - TEST-PASS | /html/semantics/popovers/popover-light-dismiss.html | Scrolling within a popover should not close the popover 
[task 2023-07-27T20:27:07.975Z] 20:27:07     INFO - TEST-UNEXPECTED-PASS | /html/semantics/popovers/popover-light-dismiss.html | Clicking inside a shadow DOM popover does not close that popover - expected FAIL
[task 2023-07-27T20:27:07.976Z] 20:27:07     INFO - TEST-INFO | expected FAIL
[task 2023-07-27T20:27:07.981Z] 20:27:07     INFO - .........
[task 2023-07-27T20:27:07.982Z] 20:27:07     INFO - TEST-OK | /html/semantics/popovers/popover-light-dismiss.html | took 4114ms

Comment 50

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

Upstream PR was closed without merging

Comment 51

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a0effe93b699
popover-light-dismiss test should have no dependency on anchor r=mbrodesser-Igalia,emilio

Comment 52

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/a0effe93b699

Comment 53

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

Upstream PR merged by moz-wptsync-bot

Comment 54

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4d64571ef929
implement light dismiss open popovers on pointer events, r=smaug

Comment 55

[Cosmin Sabou [:CosminS]]

Backed out for popovers related unexpected passes.

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=Hy9h6TkpSISXGrhF4I5mAw.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&searchStr=web&revision=82ae09837aed66d319e3280febbf971a401bd8a6&test_paths=%2Fhtml%2Fsemantics%2Fpopovers

Failure log: https://treeherder.mozilla.org/logviewer?job_id=430452286&repo=autoland

Backout link: https://hg.mozilla.org/integration/autoland/rev/ddf1641161472c3988d14e919e61e23f57fe3f8d

Full list of unexpected passes:

13795	07:22:25 INFO - TEST-UNEXPECTED-PASS | /html/semantics/popovers/light-dismiss-event-ordering.html | Tests the interactions between popover light dismiss and pointer/mouse events. eventName: pointerdown, capture: true - expected FAIL
13798	07:22:25 INFO - TEST-UNEXPECTED-PASS | /html/semantics/popovers/light-dismiss-event-ordering.html | Tests the interactions between popover light dismiss and pointer/mouse events. eventName: pointerup, capture: true - expected FAIL
13801	07:22:25 INFO - TEST-UNEXPECTED-PASS | /html/semantics/popovers/light-dismiss-event-ordering.html | Tests the interactions between popover light dismiss and pointer/mouse events. eventName: mousedown, capture: true - expected FAIL
13804	07:22:25 INFO - TEST-UNEXPECTED-PASS | /html/semantics/popovers/light-dismiss-event-ordering.html | Tests the interactions between popover light dismiss and pointer/mouse events. eventName: mouseup, capture: true - expected FAIL
13807	07:22:25 INFO - TEST-UNEXPECTED-PASS | /html/semantics/popovers/light-dismiss-event-ordering.html | Tests the interactions between popover light dismiss and pointer/mouse events. eventName: click, capture: true - expected FAIL
13810	07:22:25 INFO - TEST-UNEXPECTED-PASS | /html/semantics/popovers/light-dismiss-event-ordering.html | Tests the interactions between popover light dismiss and pointer/mouse events. eventName: pointerdown, capture: false - expected FAIL
13813	07:22:25 INFO - TEST-UNEXPECTED-PASS | /html/semantics/popovers/light-dismiss-event-ordering.html | Tests the interactions between popover light dismiss and pointer/mouse events. eventName: pointerup, capture: false - expected FAIL
13816	07:22:25 INFO - TEST-UNEXPECTED-PASS | /html/semantics/popovers/light-dismiss-event-ordering.html | Tests the interactions between popover light dismiss and pointer/mouse events. eventName: mousedown, capture: false - expected FAIL
13819	07:22:25 INFO - TEST-UNEXPECTED-PASS | /html/semantics/popovers/light-dismiss-event-ordering.html | Tests the interactions between popover light dismiss and pointer/mouse events. eventName: mouseup, capture: false - expected FAIL
13822	07:22:25 INFO - TEST-UNEXPECTED-PASS | /html/semantics/popovers/light-dismiss-event-ordering.html | Tests the interactions between popover light dismiss and pointer/mouse events. eventName: click, capture: false - expected FAIL
13825	07:22:25 INFO - TEST-UNEXPECTED-PASS | /html/semantics/popovers/light-dismiss-event-ordering.html | Tests the order of pointer/mouse events during popover light dismiss. - expected FAIL
14043	07:22:39 INFO - TEST-UNEXPECTED-PASS | /html/semantics/popovers/popover-attribute-basic.html | Changing a popover from auto to manual (via attr), and then auto during 'beforetoggle' works - expected FAIL
14050	07:22:39 INFO - TEST-UNEXPECTED-PASS | /html/semantics/popovers/popover-attribute-basic.html | Changing a popover from auto to invalid (via attr), and then auto during 'beforetoggle' works - expected FAIL
14057	07:22:39 INFO - TEST-UNEXPECTED-PASS | /html/semantics/popovers/popover-attribute-basic.html | Changing a popover from auto to null (via attr), and then auto during 'beforetoggle' works - expected FAIL
14089	07:22:39 INFO - TEST-UNEXPECTED-PASS | /html/semantics/popovers/popover-attribute-basic.html | Changing a popover from manual to auto (via attr), and then auto during 'beforetoggle' works - expected FAIL
14122	07:22:39 INFO - TEST-UNEXPECTED-PASS | /html/semantics/popovers/popover-attribute-basic.html | Changing a popover from auto to manual (via idl), and then auto during 'beforetoggle' works - expected FAIL
14129	07:22:39 INFO - TEST-UNEXPECTED-PASS | /html/semantics/popovers/popover-attribute-basic.html | Changing a popover from auto to invalid (via idl), and then auto during 'beforetoggle' works - expected FAIL
14182	07:22:39 INFO - TEST-UNEXPECTED-PASS | /html/semantics/popovers/popover-attribute-basic.html | Changing a popover from manual to auto (via idl), and then auto during 'beforetoggle' works - expected FAIL
15136	07:23:55 WARNING - ValueError: Selector '#\69 \6e \6e \65 \72 \50 \6f \70 \6f \76 \65 \72 \54 \6f \67 \67 \6c \65 ' matches no elements
15293	07:23:59 INFO - TEST-UNEXPECTED-PASS | /html/semantics/popovers/popover-light-dismiss-with-anchor.tentative.html | Clicking on anchor element (that isn't an invoking element) shouldn't prevent its popover from being closed - expected FAIL

Comment 56

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/583f2081d1ac
Backed out changeset 4d64571ef929 for popovers related unexpected passes. CLOSED TREE

Comment 57

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/47cc6b5e8fa4
implement light dismiss open popovers on pointer events, r=smaug

Comment 58

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/47cc6b5e8fa4

Comment 59

[alexander :surkov (:asurkov)]

marking as fixed, bug 1836263 will handle esc key.

Comment 60

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

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/42211 for changes under testing/web-platform/tests

Comment 61

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

Upstream PR merged by moz-wptsync-bot