1867743 - [popover] support popover=hint

Status: RESOLVED FIXED

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

Description

[zsun]

Spec in discussion:
https://github.com/whatwg/html/pull/9778

Test affected:
popover-types-with-hints.tentative.html

Comment 1

[Sebastian Zartner [:sebo]]

Once prototyped, this should be added to https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Experimental_features.

Sebastian

Comment 2

[Keith Cirkel [:keithamus]]

Attachment: Bug 1867743 - Part 7: Implement popover=hint r=dom-core,smaug

Comment 3

[Keith Cirkel [:keithamus]]

Attachment: Bug 1867743 - Part 1: Remove AddTo/RemoveFromAutoPopoverList dead code r?#dom-core

These methods are never used in the codebase.

Comment 4

[Keith Cirkel [:keithamus]]

Attachment: Bug 1867743 - Part 2: Add spec comments for popover related methods r?#dom-core

Adding code comments reflecting each line of the spec helps us confirm
behaviour adheres to existing spec, highlights where we're diverging,
and also shows us where we need to introduce new behaviour. It also
provides a snapshot - if the spec introduces new prose we can easily see
where we're out of date.

This commit has no functional changes, it's just code comments.

Comment 5

[Keith Cirkel [:keithamus]]

Attachment: Bug 1867743 - Part 3: Split out CloseEntirePopoverList, HidePopoverStackUntil r?#dom-core

This splits out the HidePopoverStackUntil function from HideAllPopoversUntil.
Splitting this out isn't strictly necessary as it's only called once at current,
but as we support popover=hint it'll need to be called twice, and having it
split out also makes it clearer when checking our code against the spec.

As a consequence of this split, the closeAllOpenPopovers lambda also
needs to be split into a function - CloseEntirePopoverList. This
is a separate algorithm in the spec, but also it's called from both
HidePopoverStackUntil and HideAllPopoversUntil.

Comment 6

[Keith Cirkel [:keithamus]]

Attachment: Bug 1867743 - Part 4: Introduce PopoverOpenedInMode r?#dom-core

PopoverOpenedInMode forces popovers to have a fixed state during their opened
lifetime, which may be different from the attribute state. They can use the
PopoverAttributeState as they can only be opened in modes that match their
attribute state.

This change introduces this new mode in the minimum ally viable way -
introducing it just for the check in HideAllPopoversUntil step 7; and the
callsites where it is set.

Comment 7

[Keith Cirkel [:keithamus]]

Attachment: Bug 1867743 - Part 5: Tidy up ShowPopover code, aligning to spec r?#dom-core

Some of the steps in ShowPopover were already implemented, but in slightly
different ways compared to the current state of the spec. This commit moves
those particular lines of code around to be more aligned to the spec, in
preparation for popover=hint.

There are no functional changes in this commit, behaviour should be
identical.

Comment 8

[Keith Cirkel [:keithamus]]

Attachment: Bug 1867743 - Part 6: Refactor popover methods to no longer hardcode to Auto mode r?#dom-core

The methods CloseEntirePopoverList, HidePopoverStackUntil, AutoPopoverList, and
IsAutoPopover all either take a popover list (of hint or auto) or have
equivalent methods (e.g. HintPopoverList). In order to introduce popover=hint,
this change refactors these methods to take a PopoverAttributeState which
determines the "opened in mode" to filter the popover list by, ensuring that
these methods can act on either Auto or Hint popovers.

This change has no behavioural changes - everything should function the
same - but it is preparatory work for popover=hint.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/9b706eb0a2b2
https://hg.mozilla.org/integration/autoland/rev/298ba2b1a98f
Part 1: Remove AddTo/RemoveFromAutoPopoverList dead code r=dom-core,smaug
https://github.com/mozilla-firefox/firefox/commit/fd0d03758de4
https://hg.mozilla.org/integration/autoland/rev/9d6b3f84d113
Part 2: Add spec comments for popover related methods r=dom-core,smaug
https://github.com/mozilla-firefox/firefox/commit/de1ed576e723
https://hg.mozilla.org/integration/autoland/rev/f44c5814c8f4
Part 3: Split out CloseEntirePopoverList, HidePopoverStackUntil r=dom-core,smaug
https://github.com/mozilla-firefox/firefox/commit/e0d620ec7e5b
https://hg.mozilla.org/integration/autoland/rev/4f5ddfd1c415
Part 4: Introduce PopoverOpenedInMode r=dom-core,smaug
https://github.com/mozilla-firefox/firefox/commit/6f16921de968
https://hg.mozilla.org/integration/autoland/rev/c8d21baa3c77
Part 5: Tidy up ShowPopover code, aligning to spec r=dom-core,smaug

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/6d5ada44aabf
https://hg.mozilla.org/integration/autoland/rev/9c7385b8d7a8
Part 6: Refactor popover methods to no longer hardcode to Auto mode r=dom-core,smaug

Comment 11

[Keith Cirkel [:keithamus]]

Attachment: Bug 1867743 - Part 8: Implement accessible relations for hint popovers r=accessibility-platform-reviewers,Jamie

Comment 12

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/298ba2b1a98f
https://hg.mozilla.org/mozilla-central/rev/9d6b3f84d113
https://hg.mozilla.org/mozilla-central/rev/f44c5814c8f4
https://hg.mozilla.org/mozilla-central/rev/4f5ddfd1c415
https://hg.mozilla.org/mozilla-central/rev/c8d21baa3c77
https://hg.mozilla.org/mozilla-central/rev/9c7385b8d7a8

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/1714552e61bd
https://hg.mozilla.org/integration/autoland/rev/4f5ff3615b35
Part 7: Implement popover=hint r=dom-core,smaug

Comment 14

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

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

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/f0c9c81272c3
https://hg.mozilla.org/integration/autoland/rev/210a5405219d
Revert "Bug 1867743 - Part 7: Implement popover=hint r=dom-core,smaug" for causing linting opt failures.

Comment 16

[Serban Stanca [:SerbanS]]

Reverted this because it was causing linting opt failures.

• Revert link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/web-platform/tests/html/semantics/popovers/WEB_FEATURES.yml:0 | The WEB_FEATURES.yml file references a test that does not exist: '!popover-light-dismiss-hint.html' (MISSING-WEB-FEATURES-FILE)

Comment 17

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

Upstream PR merged by moz-wptsync-bot

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/fdb65c5972a9
https://hg.mozilla.org/integration/autoland/rev/dd0cc742426d
Part 7: Implement popover=hint r=dom-core,smaug
https://github.com/mozilla-firefox/firefox/commit/b72f9faaf445
https://hg.mozilla.org/integration/autoland/rev/30e34947d7b6
Part 8: Implement accessible relations for hint popovers r=accessibility-platform-reviewers,Jamie

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/9e0669996cc1
https://hg.mozilla.org/integration/autoland/rev/ed0c67da9bae
Revert "Bug 1867743 - Part 8: Implement accessible relations for hint popovers r=accessibility-platform-reviewers,Jamie" for causing ba failures @browser_relations_domain_popoverinvokerisdetails.js.

Comment 20

[agoloman]

Backed out for causing ba failures @browser_relations_domain_popoverinvokerisdetails.js.

• Backout link
• Push with failures
• Failure Log

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/27dc6421fb59
https://hg.mozilla.org/integration/autoland/rev/ba3bf8c36681
Part 7: Implement popover=hint r=dom-core,smaug
https://github.com/mozilla-firefox/firefox/commit/aea2f6390911
https://hg.mozilla.org/integration/autoland/rev/bcbe91ed1999
Part 8: Implement accessible relations for hint popovers r=accessibility-platform-reviewers,Jamie

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/daf8354afc54
https://hg.mozilla.org/integration/autoland/rev/3f9b4a1f848c
Revert "Bug 1867743 - Part 8: Implement accessible relations for hint popovers r=accessibility-platform-reviewers,Jamie" for causing reftest failures @ GlobalStyleSheetCache.cpp

Comment 23

[Norisz Fay [:noriszfay]]

Backed out for causing reftest failures @ GlobalStyleSheetCache.cpp

Backout link

Push with failures

Failure log

Failure log - bc

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/372945d8117d
https://hg.mozilla.org/integration/autoland/rev/4ba2c666992c
Part 7: Implement popover=hint r=dom-core,smaug
https://github.com/mozilla-firefox/firefox/commit/30e9ab3d020f
https://hg.mozilla.org/integration/autoland/rev/6293507d171b
Part 8: Implement accessible relations for hint popovers r=accessibility-platform-reviewers,Jamie

Comment 25

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/4ba2c666992c
https://hg.mozilla.org/mozilla-central/rev/6293507d171b

Comment 26

[Pascal Chevrel:pascalc]

Please request an addition to 149 release notes, thanks.
https://wiki.mozilla.org/Release_Management/Release_Notes_Nomination

Comment 27

[Keith Cirkel [:keithamus]]

Release Note Request (optional, but appreciated)
[Why is this notable]: Added support for popover=hint
[Affects Firefox for Android]: Yes
[Suggested wording]: Added support for popover=hint
[Links (documentation, blog post, etc)]: https://developer.mozilla.org/en-US/docs/Web/API/Popover_API/Using#using_hint_popover_state

Comment 28

[Pascal Chevrel:pascalc]

Note added to our 149 nightly release notes in the Web Platform section with this wording:

Added support for the HTML attribute popover="hint".

I am keeping the relnote-firefox? flag set until we include this note in our final 149 release notes, thanks.

Comment 29

[Dave Letorey]

Related issues and pull requests

• Content PR
• BCD PR
• Firefox release

Comment 30

[Keith Cirkel [:keithamus]]

Attachment: WIP: Bug 1867743 - WIP! comment out a branch

Comment 31

[BugBot [:suhaib / :marco/ :calixte]]

A patch has been attached on this bug, which was already closed. Filing a separate bug will ensure better tracking. If this was not by mistake and further action is needed, please alert the appropriate party. (Or: if the patch doesn't change behavior -- e.g. landing a test case, or fixing a typo -- then feel free to disregard this message)