1848282 - A11y word breaks out of sync with keyboard navigation

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[James Teh [:Jamie]]

STR (with the NVDA screen reader):

1. Open this test case:
   data:text/html,<input type="text" value="ab'cd">
2. Focus the text input.
3. Press home to move to the start of the text.
4. Press control+rightArrow.
   • Expected: NVDA says "cd"
   • Actual: NVDA says "ab'cd"

Impact: Screen reader users can't work out where word boundaries are in the presence of punctuation. They will be confused by double speaking and won't know where the caret has actually moved.

Even though the caret lands on "c" (so keyboard navigation treats "ab'" as one word and "cd" as another word), accessibility treats the word as "ab'cd". Accessibility and keyboard navigation must be in sync at all times with respect to how word boundaries are calculated.

Setting intl.icu4x.segmenter.enabled to false fixes this.

Comment 1

[James Teh [:Jamie]]

As I understand it, intl.icu4x.segmenter.enabled is supposed to change everything to use the new segmenter. Why then does keyboard navigation (I guess PeekOffset, etc.) seem to be using different segmentation?

Do we need to fix keyboard navigation here? Or is the correct fix to disable the icu4x segmenter for accessibility word breaking? Or something else?

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

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

Comment 3

[Makoto Kato [:m_kato]]

Hmm, I think that WordBreaker::FindWord returns different result...

This new segmenter is Nightly only. so we won't turn on release 118 until all regressions are fixed.

Comment 4

[James Teh [:Jamie]]

Note that layout has some code (in ClusterIterator, IsSelectionInlineWhitespace, IsSelectionNewline, etc.) to handle punctuation and white space. A11y needs to emulate layout word breaks, so it has similar code... but we can't reuse the same code because the layout code is entangled with layout frames. This will probably make unifying this behaviour a little tricky.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

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

Comment 6

[Makoto Kato [:m_kato]]

Hmm, Although I setup with NDVA 2023.1jp, NVDA says "ab'cd" on Firefox 117. How to reproduce this?

Comment 7

[Makoto Kato [:m_kato]]

I guess that sample is invalid. I am looking for valid sample.

Comment 8

[James Teh [:Jamie]]

The test case is valid. The tricky part here is that with intl.icu4x.segmenter.enabled set to true, the caret movement behaviour with control+rightArrow doesn't change, but what is returned by a11y does change when it shouldn't.

Comment 9

[James Teh [:Jamie]]

[Removed incorrect comment.]

Comment 10

[Makoto Kato [:m_kato]]

I can reproduce this by the following step. Original step doesn't move caret by Ctrl+Arrow.

1. Open data:text/html,<input type="text" value="ab'cd">.
2. Focus textbox
3. Blur textbox
4. Focus textbox again.
5. Input any text in textbox then remove it
6. Move caret to head
7. Ctrl+Arrow

Comment 11

[James Teh [:Jamie]]

Ah, that's because tabbing to the text box selects all text. Sorry, i was focusing with NVDA browse mode rather than tab, which does not select all.

An easier way to work around this is to tab to the text box, press left arrow, and then press control+right arrow.

Comment 12

[Makoto Kato [:m_kato]]

Ah, that's because tabbing to the text box selects all text. Sorry, i was focusing with NVDA browse mode rather than tab, which does not select all.

I see.

Hmm, "ab'cd" should be 1 word by UAX#29 spec. But nsFrameSelection (or nsTextFrame?) doesn't recognize this is 1 word. We should fix on layout side.

Comment 13

[James Teh [:Jamie]]

I just edited comment 8 and comment 9 a lot to correct them, since they were completely bogus. Sorry about that.

Comment 14

[James Teh [:Jamie]]

The layout.word_select.stop_at_punctuation and layout.word_select.stop_at_underscore prefs are probably relevant here. Should those even be relevant with icu4x?

Comment 15

[Makoto Kato [:m_kato]]

(In reply to James Teh [:Jamie] from comment #14)

The layout.word_select.stop_at_punctuation and layout.word_select.stop_at_underscore prefs are probably relevant here. Should those even be relevant with icu4x?

Yes. we guess that we should reference this even if using ICU4X.

Comment 16

[Makoto Kato [:m_kato]]

Attachment: Bug 1848282 - Part 1. Move IsPunctuation to nsUnicharUtils. r=TYLin

Remove duplication code.

Comment 17

[Makoto Kato [:m_kato]]

Attachment: Bug 1848282 - Part 2. FindWord adds an option whether segment is punctuation. r=TYLin

This fix adds stop at punctuation mode for FindWord for a11y.

Also, IS_ASCII is conflict with nsUnicharUtils.h's, so it is renamed.

Depends on D188784

Comment 18

[Makoto Kato [:m_kato]]

Attachment: Bug 1848282 - Part 3. a11y's word segmenter should reference layout.word_select preferences. r=morgan

Depends on D188785

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c800ae9f7943
Part 1. Move IsPunctuation to nsUnicharUtils. r=TYLin
https://hg.mozilla.org/integration/autoland/rev/3446c1367bfd
Part 2. FindWord adds an option whether segment is punctuation. r=TYLin
https://hg.mozilla.org/integration/autoland/rev/d1fc71cc3817
Part 3. a11y's word segmenter should reference layout.word_select preferences. r=morgan

Comment 20

[Natalia Csoregi [:nataliaCs]]

Backout for causing gv-junit failures.

• backout: https://hg.mozilla.org/integration/autoland/rev/340ff5b040db29059fa1e5f7c6c4b6ed7fb44093
• push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&selectedTaskRun=VbncKDJORHOZ1i3DCuE0AA.0&revision=d1fc71cc3817fe3529b10939ed53c1d395a346fa
• failure log: https://treeherder.mozilla.org/logviewer?job_id=430595871&repo=autoland&lineNumber=2007

Comment 21

[Makoto Kato [:m_kato]]

Attachment: Bug 1848282 - Part 4. Make layout.word_select.stop_at_punctuation atomic. r=TYLin

GeckoView runs a11y code in UI thread, not Gecko main thread. So I would like
to change layout.word_select.stop_at_punctuation to atomic.

Depends on D188786

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cbf382972a0c
Part 1. Move IsPunctuation to nsUnicharUtils. r=TYLin
https://hg.mozilla.org/integration/autoland/rev/e1f0c76f6fa7
Part 2. FindWord adds an option whether segment is punctuation. r=TYLin
https://hg.mozilla.org/integration/autoland/rev/0320eac255f1
Part 3. a11y's word segmenter should reference layout.word_select preferences. r=morgan
https://hg.mozilla.org/integration/autoland/rev/ac94b9a48562
Part 4. Make layout.word_select.stop_at_punctuation atomic. r=TYLin

Comment 23

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/cbf382972a0c
https://hg.mozilla.org/mozilla-central/rev/e1f0c76f6fa7
https://hg.mozilla.org/mozilla-central/rev/0320eac255f1
https://hg.mozilla.org/mozilla-central/rev/ac94b9a48562