1508446 - Make label accessKey formatting always use the accesskey attribute set on the label itself instead of looking up XBL binding parent

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, task, P3)

Description

[Brian Grinstead [:bgrins]]

Spinning this out of Bug 1448213 / https://phabricator.services.mozilla.com/D5686:

Note there's a semantic change here where I'm suggesting we require that callers properly set the accesskey on the label (either through [inherits] on the XBL binding parent (or by setting it directly on the label in the label[control] case). This helps with performance and simplification because:

1) We don't need to read bindingParent when formatting (which can prematurely construct XBL)
2) We don't have to handle reformatting if/when the control changes (since the label itself controls the accesskey).

The new model is that the label will always render the accessKey based on what is set on the [accesskey] attribute.

Comment 1

[Brian Grinstead [:bgrins]]

I did a try push that throws in the various cases here where the attribute is being mirrored by the label-control / basetext bindings and I'm not seeing any relevant errors, so this looks safe to remove without rewriting callers:

- https://treeherder.mozilla.org/#/jobs?repo=try&revision=9df015318f46895fda50594141647ce2a5fcdb37
- https://hg.mozilla.org/try/rev/2f9ba10f02ba78b8976a76ba55c88765bbfd0229

Comment 2

[Brian Grinstead [:bgrins]]

Attachment: Bug 1508446 - Require that [accesskey] gets set on <xul:label> to enable formatting without referencing a binding parent;r=paolo

Previously, if the accesskey attribute was missing then the label would reach up
to binding parent to find it's accesskey. In practice, bindings already do
[xbl:inherits=accesskey] to send it down to the label anyway.

The problem with this is that for controls without accesskeys, the attribute doesn't get set,
so the label will access the control from JS. This is fine for XBL, since typically
the label XBL will construct at the same time as the control, but when migrating
to Custom Elements, the label gets connected even when the control is hidden.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fab7024e7658
Require that [accesskey] gets set on <xul:label> to enable formatting;r=paolo

Comment 4

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/54812dc57a75
Backed out changeset fab7024e7658 for reftest failures in reftest/tests/layout/reftests/xul/accesskey.xul

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ef831b231f7d
Require that [accesskey] gets set on <xul:label> to enable formatting without referencing a binding parent;r=paolo

Comment 6

[Natalia Csoregi [:nataliaCs]]

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