Closed Bug 1768219 Opened 2 years ago Closed 2 years ago

5.1 - 4.72% a11yr dhtml.html / a11yr dhtml.html (Windows) regression on Fri April 22 2022

Categories

(Core :: Disability Access APIs, defect)

defect

Tracking

()

RESOLVED FIXED
102 Branch
Tracking Status
firefox-esr91 --- unaffected
firefox100 --- unaffected
firefox101 --- fixed
firefox102 --- fixed

People

(Reporter: aglavic, Assigned: Jamie)

References

(Regression)

Details

(4 keywords)

Attachments

(1 file)

Perfherder has detected a talos performance regression from push f96f2a799305a9b18690f78ba2c13d5eb7a828bf. As author of one of the patches included in that push, we need your help to address this regression.

Regressions:

Ratio Test Platform Options Absolute values (old vs new)
5% a11yr dhtml.html windows10-64-shippable-qr e10s fission stylo webrender-sw 369.05 -> 387.86
5% a11yr dhtml.html windows10-64-shippable-qr e10s fission stylo webrender-sw 371.50 -> 389.03

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests. Please follow our guide to handling regression bugs and let us know your plans within 3 business days, or the offending patch(es) will be backed out in accordance with our regression policy.

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

For more information on performance sheriffing please see our FAQ.

Flags: needinfo?(jteh)

Set release status flags based on info from the regressing bug 1395181

I'm looking into this.

Assignee: nobody → jteh

We call ActionCount() when serialising the tree for IPC here in order to determine whether to send the eActionable type. That is only used by ATK because it needs to know whether to expose the action interface.

With the "click ancestor" action, that means we now walk some ancestors for a lot of Accessibles where we didn't previously.

It does seem bad that we incur this hit. I can think of two solutions:

  1. For most Accessibles, use HasPrimaryAction. For LinkableAccessibles (TextLeaf/Image), check ActionCount. This should get us back to where we were before, but it means we can't support "click ancestor" for ATK. Once we have the cache (not that far off), this restriction will go away. (At that point, this Talos test will also get a lot worse, but we'll deal with that when it comes.)
  2. Unconditionally support the action interface for ATK. This is what we do on Windows. This might be risky, though; it's unclear how ATK clients would react.

Eitan, thoughts?

Joanie, how would Orca react if the ATK action interface is exposed for an Accessible but there are no actions; i.e. ActionCount would return 0? Do you know of any other clients that might break here? Can you see any other problems with this? Note that exposed actions can technically change over time (e.g. click listener added/removed), so this is probably more correct anyway.

Flags: needinfo?(jteh)
Flags: needinfo?(jdiggs)
Flags: needinfo?(eitan)
Has Regression Range: --- → yes

We're going to back out bug 1395181 from beta but leave it on nightly to give us a bit more time to come up with a solution here.

I would opt for option 1 just because it would bring back the status quo before this change in ATK.

Flags: needinfo?(eitan)

I just took a quick look at the Orca source code. For the most part, when the action interface is queried (queryAction), the code assumes no actions if the interface is not implemented. However, there is at least one place which assumes at least 1 action if the action interface is implemented. So, option 2 isn't feasible anyway.

Flags: needinfo?(jdiggs)

This was causing a performance regression.
We now only do this for text leaf and image Accessibles, which gets us back to where we were before bug 1395181.
This means we can't support "click ancestor" on ATK.
There weren't requests for this on ATK anyway.
In future, we can probably support this using the information in the cache.

Pushed by jteh@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cccd838fe8f6
Don't walk ancestors on all Accessibles to find an action when serialising the IPC tree. r=eeejay
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 102 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: