1897597 - IAccessibleText assumes lines breaks when switching text direction

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Leonard]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:126.0) Gecko/20100101 Firefox/126.0

Steps to reproduce:

1. With NVDA as a screen reader on Windows, open Firefox
2. On the address bar, type: data:text/html, <textarea />
3. In the text area, type: The first hebrew letter is א, the second is ב.
4. Use up and down arrows to move through the text.

Actual results:

NVDA fails to read the hebrew characters in the text.
Inspection with IAccessibleTextObject.textAtOffset reveals that erroneous line breaks are assumed around the hebrew characters. Therefore when NVDA retrieves offsets with textAtOffset using IA2_TEXT_BOUNDARY_LINE, it receives partial lines.

Expected results:

NVDA reads the text as provided, including Hebrew characters.

Comment 1

[James Teh [:Jamie]]

Morgan, I vaguely recall us thinking we would probably hit a problem like this with bidi text, but I can't remember whether we got any further than that thought.

When a single text node wraps across lines, it gets split into continuations. Thus, a11y::TextLeafRange treats continuations as line breaks. I haven't looked at the layout frame tree yet, but I'm fairly sure changes in text direction cause text to be split into continuations as well. Thus, TextLeafRange incorrectly treats these as line breaks.

We could use a line iterator to detect this like we do in IsLocalAccAtLineStart, but I'd rather not because line iterators are complicated and potentially expensive. This led me to wonder whether there's some way to ask a continuation whether it was split because of a line break or bidi. I found two state bits of potential interest:

1. NS_FRAME_IS_FLUID_CONTINUATION: For a continuation frame, if this bit is set, then this a "fluid" continuation, i.e., across a line boundary. Otherwise it's a "hard" continuation, e.g. a bidi continuation.
2. NS_FRAME_IS_BIDI: If this bit is set, the frame itself is a bidi continuation, or is incomplete (its next sibling is a bidi continuation).

I think we might need to use both of these because it seems that a hard/non-fluid continuation can also happen for other reasons.

There's also a concept of "flow", but I don't think we can use that because nsTextFrame::NextInFlow just returns null if the next continuation isn't fluid. In contrast, I think we want to skip past non-fluid continuations to any subsequent fluid continuations. That said, I'm not entirely clear what happens if you have both non-fluid (bidi) and fluid (line wrapping) continuations in the same text node.

Assuming I'm remotely correct here, we'll need to tweak TextLeafRange to skip non-fluid continuations when it walks. There are also some spots where we check the continuation of the origin that will probably need to be tweaked as well.

Comment 2

[James Teh [:Jamie]]

I have a patch locally which mostly fixes this. However, there is an edge case it doesn't handle:

data:text/html,<meta charset="utf-8"><p contenteditable style="font-family: monospace; width: 3ch; word-break: break-word">אb בב eא

A11y thinks the first line is:

"אb בב "

It should actually be:

"אb "

The second frame doesn't have NS_FRAME_IS_FLUID_CONTINUATION, I guess because it is in fact a bidi split... but it's also a line wrap!

:jfkthame, is there some cheap way I can detect a bidi continuation which also flows onto a new line?

Comment 3

[James Teh [:Jamie]]

Attachment: Bug 1897597 part 2: Don't treat bidi split continuations as line breaks.

Continuations occur when text wraps onto a new line (fluid continuations), but they also occur when the text direction changes (non-fluid continuations).
Previously, TextLeafRange assumed that all continuations were line breaks.
We now skip non-fluid continuations when searching for the next or previous line break.

Comment 4

[James Teh [:Jamie]]

I guess one way might be to use a line iterator if a continuation is non-fluid and both it and its previous continuation have the NS_FRAME_IS_BIDI state bit. I'm still hoping for a solution that doesn't require a line iterator, though.

Comment 5

[Jonathan Kew [:jfkthame]]

It sounds to me like a line iterator really is what you want here. IIRC we added some line iterator caching a while back, so it may not be terribly expensive.

Comment 6

[James Teh [:Jamie]]

I was hoping you wouldn't say that, but... fair enough.

The problem is that we only have caching for nsBlockInFlowLineIterator, but there are frames other than block frames which can provide an nsILineIterator. The nsILineIterator for block frames doesn't cache. That means we need to use nsBlockInFlowLineIterator for block frames but nsILineIterator for everything else, which makes things complicated. I also worry there could be a case where nsILineIterator on a non-block frame gets expensive.

Comment 7

[James Teh [:Jamie]]

Attachment: Bug 1897597 part 1: Split TextLeafRange code to check whether frames are on different lines into a separate function.

There should be no functional change here.
However, we're going to need to reuse this elsewhere in the next patch.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6652a57a0bc6
part 1: Split TextLeafRange code to check whether frames are on different lines into a separate function. r=morgan
https://hg.mozilla.org/integration/autoland/rev/808efd1651fa
part 2: Don't treat bidi split continuations as line breaks. r=morgan

Comment 9

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/6652a57a0bc6
https://hg.mozilla.org/mozilla-central/rev/808efd1651fa