1689816 - Stop using nsDeckFrame in XUL <tabpanels>

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, task)

Description

[Tim Nguyen :ntim]

From bug 1559192:

MozTabpanels (https://searchfox.org/mozilla-central/rev/1133b6716d9a8131c09754f3f29288484896b8b6/toolkit/content/widgets/tabbox.js#151) could then either inherit from customElements.get("deck") or implement the same logic as deck.

Comment 1

[James Teh [:Jamie]]

Please flag someone from the a11y team (probably me) when working on this, as it could have a11y implications. I suspect the existing a11y tests should pick up any problems, but best to be safe. Thanks.

Comment 2

[Tim Nguyen :ntim]

(In reply to Tim Nguyen :ntim from comment #0)

From bug 1559192:

MozTabpanels (https://searchfox.org/mozilla-central/rev/1133b6716d9a8131c09754f3f29288484896b8b6/toolkit/content/widgets/tabbox.js#151) could then either inherit from customElements.get("deck") or implement the same logic as deck.

Inheriting from <deck> seems pretty feasible at first sight. <tabpanels> has very slight differences in methods that are overridden, but these could be put directly in <deck>. The rest is mostly extra methods.

(In reply to James Teh [:Jamie] from comment #1)

Please flag someone from the a11y team (probably me) when working on this, as it could have a11y implications. I suspect the existing a11y tests should pick up any problems, but best to be safe. Thanks.

Sure, will do. I looked further and I found these 2 potentially relevant to this bug:

• https://searchfox.org/mozilla-central/search?q=symbol:_ZN22nsAccessibilityService17DeckPanelSwitchedEPN7mozilla9PresShellEP10nsIContentP8nsIFrameS6_&redirect=false
• https://searchfox.org/mozilla-central/rev/fd853f4aea89186efdb368e759a71b7a90c2b89c/accessible/generic/Accessible.cpp#330

It seems like it's mostly about getting/firing the right offscreen states, which I expect visibility: collapse or hidden, or even display: none should hopefully do.

The rest of the nsDeckFrame a11y code can be removed in bug 1689817.

Comment 3

[James Teh [:Jamie]]

(In reply to Tim Nguyen :ntim from comment #2)

It seems like it's mostly about getting/firing the right offscreen states, which I expect visibility: collapse or hidden, or even display: none should hopefully do.

There are two worrying things here:

1. If we use visibility: hidden/collapse or display: none, I think that will cause the tabpanel (and all of its descendants) to be removed from the a11y tree. I think that's going to include the tab document. If I'm right, that's very bad. While we normally want hidden content to be removed from the a11y tree, we don't want this for background tabs, since that would cause a11y clients to lose any state they maintained for those documents. This doesn't currently happen with nsDeckFrame.
2. Even for CSS changes which cause content to be marked as off-screen for a11y (but don't remove it from the a11y tree), we don't fire an offscreen state change. Normally, this isn't needed, but it is definitely needed for tabpanels so a11y clients know which tab is active. So, we're going to need a specific notification or event for tabpanel changes, à la nsAccessibilityService::DeckPanelSwitched.

Comment 4

[James Teh [:Jamie]]

Note that these issues are covered by tests:

• accessible/tests/mochitest/events/test_statechange_tabpanels.xhtml
• accessible/tests/mochitest/states/test_visibility.xhtml

Comment 5

[Tim Nguyen :ntim]

Attachment: Bug 1689816 - Stop using nsDeckFrame in XUL <tabpanels>.

Depends on D46489

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1689816 - Implement <tabpanels> and <deck> without XUL layout. r=Gijs!,#layout!,Jamie!,morgan!

Gijs for front-end bits, layout for the new CSS properties and the
removal of nsDeckFrame / nsStackLayout, Jamie and Morgan for the a11y
changes.

As discussed in the bug, the main tricky part here is handling a11y
correctly. For <deck>, that's trivial (just use visibility: hidden to
hide the panels visually, while removing the unselected panels from the
a11y tree).

For <tabpanels> however we need to do something special. We do want to
hide stuff visually, but we want to preserve the contents in the a11y
tree.

For that, the easiest fix is introducing a new privileged CSS property
(-moz-subtree-hidden-only-visually), which takes care of not painting
the frame, but marks stuff offscreen in the accessibility tree. This is
not intended to be a property used widely.

Other than that, the changes are relatively straight-forward, though
some of the accessible/mac changes I could get a sanity-check on.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1689816 - Flush style in moveFocus. r=smaug

This is callable from JS, and nothing guarantees that style has been
updated properly before. Without this, my previous patch causes failures
in browser/base/content/test/tabdialogs/browser_tabdialogbox_focus.js,
because the nsDeckFrame used to listen to attribute changes
synchronously and now it's style-based, so if you moveFocus right after
revealing it we'd end up with the wrong answer.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/89a299b73d4a
Flush style in moveFocus. r=smaug

Comment 9

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/89a299b73d4a

Comment 10

[Emilio Cobos Álvarez (:emilio)]

There's this one test_tabbox timeout that I need to debug on macOS but otherwise this should be green-enough: https://treeherder.mozilla.org/jobs?repo=try&revision=66a019032a6b965319cd4216f651151b1eb5e504

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2f74f8f2ed19
Implement <tabpanels> and <deck> without XUL layout. r=Gijs,Jamie,morgan,preferences-reviewers,mconley,TYLin

Comment 12

[Marian-Vasile Laza]

Backed out changeset 2f74f8f2ed19 (Bug 1689816) for causing reftest failures on skip-ink-multiline-position.html.
Backout link
Push with failures <--> R1
Failure Log

Comment 13

[Emilio Cobos Álvarez (:emilio)]

That's bug 1770273 but in another test :/

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a0f86ac41bc7
Implement <tabpanels> and <deck> without XUL layout. r=Gijs,Jamie,morgan,preferences-reviewers,mconley,TYLin

Comment 15

[Atila Butkovits]

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