1722396 - Refactor DOM attribute change handling

Status: RESOLVED FIXED

(Core :: Disability Access APIs, task, P1)

Description

[Eitan Isaacson [:eeejay]]

Our current DOM attribute changed listener in DocAccessible is spaghetti code and touches on many different things. This includes changes in relations, aria-owns, states accessible attributes, and name/description changes. Some of these codepaths are specialized for specific accessible types (eg. a summary attribute is only relevant to tables).

In the past we were forced to store the old state of accessibles in the AttributeWillChange callback when we aren't able to determine for certain if an attribute change changes a state. Since we are already doing this, that technique could be used more broadly in many cases to give a proper state-change event, this will simplify and robustify our state-change code.

Comment 1

[Eitan Isaacson [:eeejay]]

Attachment: Bug 1722396 - P1: Remove DocAccessible::mARIAAttrOldValue. r?morgan!

The AttributeChanged callback includes the old value, so there isn't a
reason to store this from AttributeWillChange.

Comment 2

[Eitan Isaacson [:eeejay]]

Attachment: Bug 1722396 - P2: Move AttributeChangedImpl into LocalAccessible::DOMAttributeChanged. r?Jamie!,morgan!

Also folded ARIAAttributeChanged into DOMAttributeChanged temporarily.
Will streamline that further in the next patch where state changes are
done a bit better.

Depends on D120900

Comment 3

[Eitan Isaacson [:eeejay]]

Attachment: Bug 1722396 - P3: Compare old state bits with new ones to determine state change events. r?morgan!,Jamie!

Needed to tweak tests a bit because the code is now more descerning
wheter to fire an event or not. Will do so only if the state actually
changes.

Depends on D120901

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a1cbf6bbf0c3
P1: Remove DocAccessible::mARIAAttrOldValue. r=morgan
https://hg.mozilla.org/integration/autoland/rev/d953dc56b027
P2: Move AttributeChangedImpl into LocalAccessible::DOMAttributeChanged. r=morgan
https://hg.mozilla.org/integration/autoland/rev/cda26075ae09
P3: Compare old state bits with new ones to determine state change events. r=morgan

Comment 5

[Dorel Luca [:dluca]]

Backed out 3 changesets (bug 1722396) for Devtools failures in devtools/server/tests/browser/browser_accessibility_node_events.js. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer?job_id=346658116&repo=autoland&lineNumber=2672

There are also some BC failures:
https://treeherder.mozilla.org/logviewer?job_id=346658384&repo=autoland&lineNumber=1701

Push with failures:
https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&revision=cda26075ae09d337ca4a1aa64c0721ab6926b2d9

Backout:
https://hg.mozilla.org/integration/autoland/rev/7f3cf93f5095d10da485a393937d8dfcff492db8

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e2cf9edf5d73
P1: Remove DocAccessible::mARIAAttrOldValue. r=morgan
https://hg.mozilla.org/integration/autoland/rev/20b2120a002a
P2: Move AttributeChangedImpl into LocalAccessible::DOMAttributeChanged. r=morgan
https://hg.mozilla.org/integration/autoland/rev/52cc9f22fbd8
P3: Compare old state bits with new ones to determine state change events. r=morgan

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3fcd2aa75f63
Fix eslint. r=lint-fix CLOSED TREE

Comment 8

[Noemi Erli[:noemi_erli]]

Backed out 4 changesets (Bug 1722396) for causing multiple failures CLOSED TREE

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=842382&selectedTaskRun=bT1olzQtQ1ePfLqEyfx6uQ.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=52cc9f22fbd8c82337f859f5c53455d006576c4d

Log: https://treeherder.mozilla.org/logviewer?job_id=346807824&repo=autoland&lineNumber=1909
https://treeherder.mozilla.org/logviewer?job_id=346806816&repo=autoland&lineNumber=2690

Backout: https://hg.mozilla.org/integration/autoland/rev/a0a369ff460ea4070c674ed45116a3de473ed554

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/22f85fd34b27
P1: Remove DocAccessible::mARIAAttrOldValue. r=morgan
https://hg.mozilla.org/integration/autoland/rev/02dea4423d5d
P2: Move AttributeChangedImpl into LocalAccessible::DOMAttributeChanged. r=morgan
https://hg.mozilla.org/integration/autoland/rev/86fc719a7a94
P3: Compare old state bits with new ones to determine state change events. r=morgan

Comment 10

[Dorel Luca [:dluca]]

Backed out 3 changesets (bug 1722396) for Linting failure in accessible/tests/browser/mac/browser_aria_expanded.js. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer?job_id=347191432&repo=autoland&lineNumber=114

Push with failures:
https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&revision=86fc719a7a9402882ad267d6a5af865427d01b4a

Backout:
https://hg.mozilla.org/integration/autoland/rev/7153015cf34c31386b2cfaa09ac19bb903ea70c9

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9c723d30ef5f
P1: Remove DocAccessible::mARIAAttrOldValue. r=morgan
https://hg.mozilla.org/integration/autoland/rev/ea0639278659
P2: Move AttributeChangedImpl into LocalAccessible::DOMAttributeChanged. r=morgan
https://hg.mozilla.org/integration/autoland/rev/086278457495
P3: Compare old state bits with new ones to determine state change events. r=morgan

Comment 12

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a9d0f50e5a65
Backed out 3 changesets for causing gv-junit test failures. CLOSED TREE

Comment 13

[Marian-Vasile Laza]

Backed out 3 changesets (Bug 1722396) for causing gv-junit test failures.
Backout link
Failure Log
Push with failures - due to some build bustages, we triggered some tests on your try push.

Comment 14

[Eitan Isaacson [:eeejay]]

Oof sorry for all of these failed landings, my try attempts have been to selective and I have missed some of these failures. Hope this next one sticks..

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5bfab23542e1
P1: Remove DocAccessible::mARIAAttrOldValue. r=morgan
https://hg.mozilla.org/integration/autoland/rev/57cba437be32
P2: Move AttributeChangedImpl into LocalAccessible::DOMAttributeChanged. r=morgan
https://hg.mozilla.org/integration/autoland/rev/17cdb70e03a3
P3: Compare old state bits with new ones to determine state change events. r=morgan

Comment 16

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/5bfab23542e1
https://hg.mozilla.org/mozilla-central/rev/57cba437be32
https://hg.mozilla.org/mozilla-central/rev/17cdb70e03a3