Some tab documents completely missing from a11y tree on startup
Categories
(Core :: Disability Access APIs, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr78 | --- | unaffected |
firefox85 | --- | unaffected |
firefox86 | --- | unaffected |
firefox87 | --- | fixed |
People
(Reporter: Jamie, Assigned: Jamie)
References
(Regression)
Details
(Keywords: regression)
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
As of last week, when starting Firefox and restoring tabs from the previous session, Accessibles for tabs other than the foreground tab are sometimes completely broken. The property page Accessible is completely missing and the document (or anything in it) can't get focus.
mozregression suggests this is due to bug 493683:
12:19.47 INFO: Last good revision: 37cbc73b07c3e0b8d9441bea2beae71ecdc1dd14
12:19.47 INFO: First bad revision: 2c9004c95a7a7d39c2c1417b66fda37892398263
12:19.47 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=37cbc73b07c3e0b8d9441bea2beae71ecdc1dd14&tochange=2c9004c95a7a7d39c2c1417b66fda37892398263
I can't fathom how those patches could cause this, but I guess I'll start digging. :)
Comment 1•4 years ago
|
||
Set release status flags based on info from the regressing bug 493683
Assignee | ||
Comment 2•4 years ago
|
||
Wow. This is a mind boggler.
- We process pending insertions and start handling the insertion of a XULTabAccessible.
- BindToParent gets called for this XULTabAccessible.
- As part of that, we query the LABEL_FOR relation (to determine whether we need to set the eHasNameDependent flag).
- That results in a call to XULTabAccessible::RelationByType, which calls nsIDOMXULRelatedElement::getRelatedElement. Unfortunately, getRelatedElement is implemented in JS.
- When we call into JS, other stuff runs, causing the insertion of a tabpanel.
- A11y ends up dropping this insertion, probably because it was processing other insertions and mContentInsertions was mutated while it was being iterated.
I see a few potential solutions here:
- hack: Change Accessible::BindToParent to avoid checking LABEL_FOR for a XULTabAccessible.
- Change Accessible::BindToParent to specifically call RelatedAccIterator for at aria-labelledby, instead of using the LABEL_FOR relation. This doesn't handle other cases of LABEL_FOR and is thus less versatile, but we (probably) don't care about other cases of LABEL_FOR at this stage anyway.
- Change XULTabAccessible::RelationByType to do the calculation itself instead of using nsIDOMXULRelatedElement.
- Change XUL tabpanels to use aria-labelledby instead of custom C++ code. This could be tricky because the tabs don't have ids. Also, the tabpanels don't seem to have a name right now, despite the relation, but using aria-labelledby would give them a name, which might impact AT.
- Remove the LABEL_FOR relation on XULTabAccessible. Might impact AT.
- Fix the a11y engine to be more tolerant of insertions queued during an insertion. For example, swap mContentInsertions into a temporary map before processing. That said, running JS during a tree update feels pretty fragile to me.
Eitan, any thoughts/preferences?
Assignee | ||
Comment 3•4 years ago
|
||
Personally, I think I'm leaning towards option 2 at this stage, though it's less clean than I'd like.
Updated•4 years ago
|
Comment 4•4 years ago
|
||
Huh. With no logical justification, I can live with options 1, 3 and 6 :)
I think 6 is risky, might have some other kinds of fallout, so not sure if it is the best idea, although it is the most "correct".
I like options 1 and 3 because they limit the fix/hack to XULTabAccessible.
Assignee | ||
Comment 5•4 years ago
|
||
Pushed by jteh@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/02335cb3c1af Move pending a11y content insertions into a temporary data structure before processing them. r=eeejay
Comment 7•4 years ago
|
||
bugherder |
Assignee | ||
Updated•3 years ago
|
Updated•3 years ago
|
Description
•