Closed Bug 1474814 Opened Last year Closed Last year

[e10s a11y] Focused state not updated when focus moves

Categories

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

All
Windows
defect

Tracking

()

VERIFIED FIXED
mozilla63
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- verified
firefox61 --- wontfix
firefox62 --- verified
firefox63 --- verified

People

(Reporter: Jamie, Assigned: Jamie)

Details

(Keywords: regression)

Attachments

(1 file)

Reported by VFO and causes problems for JAWS.

TL;DR: When focus moves but no other update occurs, the handler cache doesn't get invalidated.

STR (with NVDA):
We use the NVDA python console here to query the buffer. NVDA itself is out-of-process, so doesn't use the handler cache, but the buffer is in-process and does use it. It just so happens that NVDA doesn't care about the focused state reported in the buffer.
1. Open this URl:
data:text/html,<input type="checkbox" id="c"><input type="radio" id="r">
2. Press control+home to move to the check box.
3. Open the NVDA Python console with NVDA+control+z.
4. Paste the following (as a single line) and press enter:
import controlTypes; review.expand("character"); f = review.getTextWithFields(); f[1].field["IAccessible2::attribute_id"], controlTypes.STATE_FOCUSED in f[1].field["states"]
Result: (u'c', False)
5. Alt+tab back to Firefox without closing the console (so the captured review position will still be on the check box).
6. Press space to check the check box.
7. Alt+tab back to the console.
8. Execute the same command again  (press up arrow, enter).
Expected: (u'c', False)
Actual: (u'c', True)
Observation: The state changed, invalidating the cache. However, since we've alt+tabbed out, this doesn't actually have the focus any more, so the focused state shouldn't be present; we should see False, not True.
That alone might be forgivable; someone's not likely to query the focused state when focused in a completely different window. However:
9. Alt+tab back to Firefox without closing the console (so the captured review position will still be on the check box).
10. Press r to move to the radio button.
11. Alt+tab back to the console.
12. Execute the same command again  (press up arrow, enter).
Expected: (u'c', False)
Actual: (u'c', True)
Observation: If anything, the radio button might report focused. The check box should definitely not report focused at this point, but it does.

We don't fire a state change for the focused state, since that's somewhat redundant given we fire a focus event. However, the cache doesn't get invalidated for focus events. :(

Marking as a regression since this worked just fine before e10s.
Flagging 62 and 60ESR as affected since this bug is present in both. Does also affect 61, but I don't think this warrants a respin of an 61 dot release. But it is a candidate for a 62 beta uplift, and maybe a 60ESR backport.
Comment on attachment 8991223 [details]
Bug 1474814: Invalidate the AccessibleHandler cache on focus events.

https://reviewboard.mozilla.org/r/256184/#review263032

Straight-forward fix. :)
Attachment #8991223 - Flags: review?(mzehe) → review+
(In reply to James Teh [:Jamie] from comment #0)
> STR (with NVDA):
> We use the NVDA python console here to query the buffer. NVDA itself is
> out-of-process, so doesn't use the handler cache, but the buffer is
> in-process and does use it. It just so happens that NVDA doesn't care about
> the focused state reported in the buffer.
> 1. Open this URl:
> data:text/html,<input type="checkbox" id="c"><input type="radio" id="r">
> 2. Press control+home to move to the check box.
> 3. Open the NVDA Python console with NVDA+control+z.
> 4. Paste the following (as a single line) and press enter:
> import controlTypes; review.expand("character"); f =
> review.getTextWithFields(); f[1].field["IAccessible2::attribute_id"],
> controlTypes.STATE_FOCUSED in f[1].field["states"]
> Result: (u'c', False)

So far, so good.

> 5. Alt+tab back to Firefox without closing the console (so the captured
> review position will still be on the check box).
> 6. Press space to check the check box.
> 7. Alt+tab back to the console.
> 8. Execute the same command again  (press up arrow, enter).
> Expected: (u'c', False)
> Actual: (u'c', True)
> Observation: The state changed, invalidating the cache. However, since we've
> alt+tabbed out, this doesn't actually have the focus any more, so the
> focused state shouldn't be present; we should see False, not True.

This still yields (u'c', True) for me.

> That alone might be forgivable; someone's not likely to query the focused
> state when focused in a completely different window. However:
> 9. Alt+tab back to Firefox without closing the console (so the captured
> review position will still be on the check box).
> 10. Press r to move to the radio button.
> 11. Alt+tab back to the console.
> 12. Execute the same command again  (press up arrow, enter).
> Expected: (u'c', False)
> Actual: (u'c', True)
> Observation: If anything, the radio button might report focused. The check
> box should definitely not report focused at this point, but it does.

This, also, yields (u'c', True) for me. :( I definitely installed the try build, removing any previous Nightly, but it appears the patch doesn't actually cause the cache to be invalidated. Or something else is amiss. I am going to try Brett's example with JAWS next.
OK, all good with JAWS. The patch does fix the bug, it's just that the NVDA steps aren't reflecting that. This patch can land.
Pushed by mzehe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cbf89941ccff
Invalidate the AccessibleHandler cache on focus events. r=MarcoZ
(In reply to Marco Zehe (:MarcoZ) from comment #5)
> OK, all good with JAWS. The patch does fix the bug, it's just that the NVDA
> steps aren't reflecting that. This patch can land.

That'd be because my steps were bogus. I wanted a more programmatic (and public) way to reproduce this. Unfortunately, I neglected to realise that NVDA doesn't update a node in its buffer just because the focus changes (because it doesn't need to), and even if it did, it fetches a fresh copy of the accessible when it updates, thus avoiding the stale cache. Sorry about that. That's what I get for trying to be clever. :(

Unfortunately, I can't publish the test case from VFO here. The best we can do is verify that the patch fixes the reported issue from VFO, which Marco did in comment 5.
https://hg.mozilla.org/mozilla-central/rev/cbf89941ccff
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Verified fixed in Firefox 63.0a1 (20180712220034). Please request uplift. :)
Status: RESOLVED → VERIFIED
Comment on attachment 8991223 [details]
Bug 1474814: Invalidate the AccessibleHandler cache on focus events.

Approval Request Comment
[Feature/Bug causing the regression]: E10s Windows accessibility.
[User impact if declined]: Users of the JAWS screen reader will receive no feedback when navigating to form controls in some cases.
[Is this code covered by automated tests?]: No, because we don't have a mechanism for Windows platform a11y testing.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: One line patch which invalidates a cache based on an additional event.
[String changes made/needed]: None.

ESR:
If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is extremely confusing for users of the JAWS screen reader and is a regression compared to ESR 52.
Fix Landed on Version: 63, but also requesting uplift to 62.
Attachment #8991223 - Flags: approval-mozilla-esr60?
Attachment #8991223 - Flags: approval-mozilla-beta?
Comment on attachment 8991223 [details]
Bug 1474814: Invalidate the AccessibleHandler cache on focus events.

Fix for a regression in form controls. Verified in Nightly. Let's uplift for beta 9.
Attachment #8991223 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 8991223 [details]
Bug 1474814: Invalidate the AccessibleHandler cache on focus events.

Improves life for JAWS users moving from ESR52 to ESR60. Approved for ESR 60.2.
Attachment #8991223 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
Flags: qe-verify+
I tested on Firefox Beta 62.0b8 and could not navigate through checkboxes using "x" key with JAWS.
Spoked with Marco and he will help with the verification on the latest Beta and ESR60.
Flags: needinfo?(mzehe)
Verified fixed on 62.0b9. An ESR 60.2.0 is not yet built.
Flags: needinfo?(mzehe)
Verified fixed in Firefox 60.2.2 ESR.
The bug was verified on all fixed FF versions. Removing the qe-verify+ flag.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.