Cache LABELLED_BY relation
Categories
(Core :: Disability Access APIs, task)
Tracking
()
Tracking | Status | |
---|---|---|
firefox105 | --- | fixed |
People
(Reporter: morgan, Assigned: morgan)
References
(Blocks 1 open bug)
Details
(Whiteboard: [ctw-m2])
Attachments
(4 files, 2 obsolete files)
This is the relation Windows depends on the most, so we should cache this first :)
Assignee | ||
Comment 1•1 year ago
•
|
||
Last CtW meeting we discussed caching one of the label for/labelled by pair and computing the reciprocal relation in the parent process.
I thought about this some more today and am confused about how we handle the "attribute removed" case.
Say, for example, we have the following:
<div id="d1">hello</div>
<button aria-labelledby="d1"></button>
Let's also assume we're caching the labelled by relation and computing the label for relation.
We'll update our cache with a DeleteEntry()
for labelled by on the button, but how do we know (in parent) that we need to remove the div's label for relation? Outside of DOMAttributeChanged
I don't think we have access to the old value of aria-labelledby
...
:Jamie do you have thoughts on this? I don't really see a way forward here, apart from caching both relations independently (then, when we see aria-labelledby
removed in DOMAttributeChanged
, we'd use the old value to somehow fetch the acc(s) that used to act as labels, and push DeleteEntry()
s for their label for relations too. I imagine there's a hash-map-y way around this, but I'm just not seeing it right now :(
Also: I think this problem also exists if we end up caching the label for relation instead, if that node gets destroyed, or its ID changes, we still need to be able to update the reciprocal relation.
Comment 2•1 year ago
|
||
Hmm. Tricky.
My initial thinking was that we have the old value in mCachedFields, so we can just use that to compare against. Unfortunately, the cache update logic is isolated to AccAttributes::Update, which doesn't (and shouldn't) know anything about RemoteAccessibles, relations, etc.
Moving the update code into RemoteAccessibleBase::ApplyCache would make this easier, but it's also horrible and will probably require yucky messing with making other AccAttributes things public or friend classes or the like. On the flip side, it would avoid extra hash lookups.
Another alternative could be to have AccAttributes::Update take a lambda which takes the key being updated/removed, giving the caller the chance to do some pre-processing if that key is present. If this lambda sees labelledBy, it would remove the labelFor data for the target Accessibles for the pre-update labelledBy (i.e. the labelledBy still in mCachedFields). It would also set a flag so it knows to update labelFor on the new targets.
A final option could be to check for the presence of labelledBy in aFields in RemoteAccessibleBase::ApplyCache before we call mCachedFields->Update. If present (either an update or a delete), we remove the labelFor data for the target Accessibles for the pre-update labelledBy (i.e. the labelledBy still in mCachedFields). We then call mCachedFields->Update. After that, we update the labelFor on the new labelledBy targets. This does involve additional hash lookups for every cache update, though. I'm not sure if that is premature optimisation or not.
Assignee | ||
Comment 3•11 months ago
|
||
Assignee | ||
Comment 4•11 months ago
|
||
Depends on D151881
Updated•11 months ago
|
Updated•11 months ago
|
Assignee | ||
Comment 5•11 months ago
|
||
Updated•11 months ago
|
Assignee | ||
Comment 6•11 months ago
|
||
Assignee | ||
Comment 7•11 months ago
|
||
Depends on D152100
Assignee | ||
Comment 8•11 months ago
|
||
Depends on D152101
Updated•11 months ago
|
Updated•11 months ago
|
Updated•11 months ago
|
Updated•11 months ago
|
Updated•11 months ago
|
Updated•11 months ago
|
Updated•11 months ago
|
Updated•11 months ago
|
Updated•11 months ago
|
Updated•11 months ago
|
Updated•11 months ago
|
Updated•11 months ago
|
Updated•11 months ago
|
Updated•11 months ago
|
Pushed by mreschenberg@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9309568a622d [Part 1] Add cache domains, caching logic for LABEL_FOR and LABELLED_BY relations r=Jamie https://hg.mozilla.org/integration/autoland/rev/8a7e2374e0b2 [Part 2] Add implicit relations map, pre- and post-update map processing in ApplyCache r=Jamie https://hg.mozilla.org/integration/autoland/rev/14e4b5310a9a [Part 3] Make remote ::Relations and ::RelationByType functions to use cache r=Jamie https://hg.mozilla.org/integration/autoland/rev/c0cdd3e2efa2 [Part 4] Add tests for relations caching r=Jamie
Comment 10•11 months ago
|
||
Backed out for causing build bustages.
Backout link: https://hg.mozilla.org/integration/autoland/rev/4eeee5a6274709c9aad68cc34c03631da9c57d2e
Failure log: https://treeherder.mozilla.org/logviewer?job_id=385502594&repo=autoland&lineNumber=9976
Updated•11 months ago
|
Updated•11 months ago
|
Comment 11•10 months ago
|
||
Pushed by mreschenberg@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ffbb26d2a3c1 [Part 1] Add cache domains, caching logic for LABEL_FOR and LABELLED_BY relations r=Jamie https://hg.mozilla.org/integration/autoland/rev/5fff79016cb6 [Part 2] Add implicit relations map, pre- and post-update map processing in ApplyCache r=Jamie https://hg.mozilla.org/integration/autoland/rev/f44a9d0f46ab [Part 3] Make remote ::Relations and ::RelationByType functions to use cache r=Jamie https://hg.mozilla.org/integration/autoland/rev/63a4b92b5453 [Part 4] Add tests for relations caching r=Jamie
Assignee | ||
Comment 13•10 months ago
|
||
clean try: https://treeherder.mozilla.org/jobs?repo=try&selectedTaskRun=WZhEYKPGSJOfmZQLBoQvcA.0&resultStatus=success%2Ctestfailed%2Cbusted%2Cexception%2Cusercancel%2Crunning%2Cpending%2Crunnable&revision=d552ef44da1dc65906e5d440d7743e2d0c6d13a9
(one failure is unrelated, known intermittent)
Comment 14•10 months ago
|
||
Pushed by mreschenberg@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6de77a6f3d23 [Part 1] Add cache domains, caching logic for LABEL_FOR and LABELLED_BY relations r=Jamie https://hg.mozilla.org/integration/autoland/rev/570aa988c8c8 [Part 2] Add implicit relations map, pre- and post-update map processing in ApplyCache r=Jamie https://hg.mozilla.org/integration/autoland/rev/dd760c44c8ba [Part 3] Make remote ::Relations and ::RelationByType functions to use cache r=Jamie https://hg.mozilla.org/integration/autoland/rev/063973343a0e [Part 4] Add tests for relations caching r=Jamie
Comment 15•10 months ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6de77a6f3d23
https://hg.mozilla.org/mozilla-central/rev/570aa988c8c8
https://hg.mozilla.org/mozilla-central/rev/dd760c44c8ba
https://hg.mozilla.org/mozilla-central/rev/063973343a0e
Description
•