1769586 - Implement ARIA element reflection

Status: RESOLVED FIXED

(Core :: Disability Access APIs, enhancement)

Description

[Manuel Rego Casasnovas]

Steps to reproduce:

After the work for simple attributes have been done (see https://bugzilla.mozilla.org/show_bug.cgi?id=1628418), the next step would be to implement IDREF element reflection.

HTML PR spec: https://github.com/whatwg/html/pull/3917
WPT test: https://github.com/web-platform-tests/wpt/blob/master/dom/nodes/aria-element-reflection.tentative.html

JFYI, Chromium (https://bugs.chromium.org/p/chromium/issues/detail?id=981423) and WebKit (https://bugs.webkit.org/show_bug.cgi?id=196843) already have an implementation.

Comment 1

[James Teh [:Jamie]]

https://github.com/webcompat/web-bugs/issues/112375 is covered by bug 1785412, not this one. Note that this part of ARIA reflection is implemented, but not shipped yet; see bug 1785412 for the reasons.

Comment 2

[Dennis Schubert [:denschub]]

Ah, thanks!

Comment 4

[James Teh [:Jamie]]

Explainer: https://wicg.github.io/aom/aria-reflection-explainer.html#reflecting-element-references
Spec:
https://w3c.github.io/aria/#ARIAMixin
https://html.spec.whatwg.org/multipage/common-dom-interfaces.html#reflecting-content-attributes-in-idl-attributes:element

1. Bug 1823757 implements the DOM concepts of explicitly set attr-element and attr-associated element. However, some ARIA properties are an array of elements. We'll need to extend DOM to handle attr-element arrays. See dom::Element::ExplicitlySetAttrElement, GetExplicitlySetAttrElement, ClearExplicitlySetAttrElement, GetAttrAssociatedElement, etc.
2. Bug 1879255 implements a11y engine support for a single explicitly set attr-element. We'll need to extend that to handle attr-element arrays. That should be fairly straightforward. See DocAccessible::AddDependentElementsFor, DocAccessible::RemoveDependentElementsFor, and the mIsWalkingDependentElements bits in RelatedAccIterator.
3. The a11y engine will also need to be taught how to calculate ARIA relations from explicitly set attr-elements. RelatedAccIterator already handles the reverse relations thanks to bug 1879255. The forward side uses IDRefsIterator, which only knows about ids. We'll need to extend (and ideally rename) IDRefsIterator, similar to what was done for RelatedAccIterator.
4. ElementInternals doesn't support explicitly set attr-elements at all, neither singular nor array. We'll need to extend it to handle this.
5. We'll need to add the ARIA element attributes to ARIAMixin.webidl and implement them in dom::Element and ElementInternals.
6. We'll need to ensure that setting an ARIA content attribute clears the explicitly set attr-element. We do this here for popovertarget. We'll probably need to do this in dom::Element for ARIA attributes, though.
7. Finally, we'll need to teach the a11y engine how to get attr-elements from ElementInternals, in all places where we do this for normal elements.

Comment 5

[Eitan Isaacson [:eeejay]]

Attachment: Bug 1769586 - P1: Implement ARIA element reflection in Element and ElementInternals. r?peterv

Comment 6

[Eitan Isaacson [:eeejay]]

Attachment: Bug 1769586 - P2: Implement a11y support of ARIA element reflection. r?Jamie

Depends on D209767

Comment 7

[Eitan Isaacson [:eeejay]]

Attachment: Bug 1769586 - P3: Rename IDRefsIterator AssociatedElementsIterator. r?Jamie

The iterator uses both explicitly set element attributes and content attributes
with IDs to iterate over all associated elements

Depends on D209768

Comment 8

[Woody Li]

Dependencies are both resolved, any timeline for this?

Comment 9

[Eitan Isaacson [:eeejay]]

(In reply to Woody Li from comment #8)

Dependencies are both resolved, any timeline for this?

Waiting on https://github.com/web-platform-tests/wpt/pull/48058. Once it is merged I can queue these patches.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/049fd286ce0f
P1: Implement ARIA element reflection in Element and ElementInternals. r=peterv
https://hg.mozilla.org/integration/autoland/rev/b3264bc533e2
P2: Implement a11y support of ARIA element reflection. r=Jamie
https://hg.mozilla.org/integration/autoland/rev/d684010261d6
P3: Rename IDRefsIterator AssociatedElementsIterator. r=Jamie

Comment 11

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/049fd286ce0f
https://hg.mozilla.org/mozilla-central/rev/b3264bc533e2
https://hg.mozilla.org/mozilla-central/rev/d684010261d6

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Backed out from Beta for 132.0b9 for the perf regression in bug 1920082.
https://hg.mozilla.org/releases/mozilla-beta/rev/e25410bb1418bffb204a8d18b356ef6c6a445880

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Backed out for 133+ also per bug 1920082 comment 18.
https://hg.mozilla.org/integration/autoland/rev/35a3e4d687e64232d2cf88597ce63ab543d48a9a

Comment 14

[Andra Esanu (needinfo me)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #13)

Backed out for 133+ also per bug 1920082 comment 18.
https://hg.mozilla.org/integration/autoland/rev/35a3e4d687e64232d2cf88597ce63ab543d48a9a

Hi, could you please confirm if this backout can be the cause for the following browsertime improvement? Some jobs are failing to run and it is hard for us to tell since we don't have results to display on the graph. Please needinfo me. Thank you!

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)	Performance Profiles
11%	speedometer3 Charts-observable-plot/Dotted/Sync	android-hw-a55-14-0-android-aarch64-shippable-qr	webrender	22.37 -> 19.92
10%	speedometer3 Charts-observable-plot/Dotted/Sync	android-hw-a55-14-0-aarch64-shippable	webrender	22.26 -> 19.94
10%	speedometer3 Charts-observable-plot/Dotted/Sync	android-hw-a55-14-0-aarch64-shippable	webrender	22.03 -> 19.88
9%	speedometer3 Charts-observable-plot/Dotted/Sync	android-hw-a55-14-0-android-aarch64-shippable-qr	webrender	21.97 -> 19.90
7%	speedometer3 Charts-observable-plot/Dotted/total	android-hw-a55-14-0-aarch64-shippable	webrender	34.14 -> 31.70
...	...	...	...	...	...
3%	speedometer3 TodoMVC-jQuery/CompletingAllItems/Sync	linux1804-64-nightlyasrelease-qr	fission webrender	170.41 -> 165.46	Before/After

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a sheriff to do that for you.

You can run these tests on try with ./mach try perf --alert 2563

For more information on performance sheriffing please see our FAQ.

Comment 15

[Eitan Isaacson [:eeejay]]

Yes, that looks like the inverse of the regression.

Comment 16

[Peter Van der Beken [:peterv]]

Attachment: Bug 1769586 - Small build bustage fix: replace ArrayLength with std::size. r?Jamie!

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f543f54d90a0
P1: Implement ARIA element reflection in Element and ElementInternals. r=peterv
https://hg.mozilla.org/integration/autoland/rev/486c9854c11d
P2: Implement a11y support of ARIA element reflection. r=Jamie
https://hg.mozilla.org/integration/autoland/rev/ced88b84eef3
P3: Rename IDRefsIterator AssociatedElementsIterator. r=Jamie
https://hg.mozilla.org/integration/autoland/rev/1125ec697fdd
Small build bustage fix: replace ArrayLength with std::size. r=Jamie

Comment 18

[amarc]

Backed out for causing dt failures @ Cell.h

• Backout link
• Push with failures
• Failure Log
• Failure line:

Assertion failure: this->flags() == 0, at /builds/worker/checkouts/gecko/js/src/gc/Cell.h:776

————————————————————————————
Backed out for causing wpt failures @ aria-element-reflection-labelledby.html

• Push with failures
• Failure Log
• Failure line:

TEST-UNEXPECTED-PASS | /html/dom/aria-element-reflection-labelledby.html | Setting ariaLabelledByElements should determine the computed label for the labelled element - expected FAIL

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/49f68df69db0
P1: Implement ARIA element reflection in Element and ElementInternals. r=peterv
https://hg.mozilla.org/integration/autoland/rev/446faf4cccab
P2: Implement a11y support of ARIA element reflection. r=Jamie
https://hg.mozilla.org/integration/autoland/rev/d9ff02e6c435
P3: Rename IDRefsIterator AssociatedElementsIterator. r=Jamie
https://hg.mozilla.org/integration/autoland/rev/dc6aa76ec6c5
Small build bustage fix: replace ArrayLength with std::size. r=Jamie

Comment 20

[amarc]

Backed out for causing wpt failures @ property-reflection.html

• Backout link
• Push with failures
• Failure Log
• Failure line:

TEST-UNEXPECTED-FAIL | /shadow-dom/reference-target/tentative/property-reflection.html | button.ariaLabelledByElements has reflection behavior ReflectsHostInArray when pointing to button with reference targetappendTestDeclaratively - assert_array_equals: value is null, expected array

Comment 21

[Peter Van der Beken [:peterv]]

The reason that that test fails is because upstream wpt undid a fix by Eitan, https://github.com/web-platform-tests/wpt/pull/47914 undoes the change from https://github.com/web-platform-tests/wpt/pull/48058. These tests only run conditionally when the properties are exposed (not sure why?), so the test only started failing again when we relanded the patches from this bug.

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0b34774ca74a
P1: Implement ARIA element reflection in Element and ElementInternals. r=peterv
https://hg.mozilla.org/integration/autoland/rev/1207244e3c60
P2: Implement a11y support of ARIA element reflection. r=Jamie
https://hg.mozilla.org/integration/autoland/rev/ee30c11cf16a
P3: Rename IDRefsIterator AssociatedElementsIterator. r=Jamie

Comment 23

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/0b34774ca74a
https://hg.mozilla.org/mozilla-central/rev/1207244e3c60
https://hg.mozilla.org/mozilla-central/rev/ee30c11cf16a