-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Description
I don't have a repro recipe for this handy, but it's a bug I believe is there based on reading the code, and I'm filing the issue so I have something to link to in a comment.
The bug is the same as #122477, but limited to the case where there is some LTR text and also some RTL text near the cursor. Specifically:
-
The algorithm we have for [TextPainter.getOffsetForCaret], in [TextPainter._getMetricsFromDownstream] and [TextPainter._getMetricsFromUpstream], takes exponentially longer and longer ranges of code units and asks SkParagraph for the text boxes contained in those ranges.
-
Those boxes are returned in left-to-right order, not logical text order. (Thanks to @tgucio for spotting this possibility.)
Even the left-to-right order is only from empirical observation; we haven't pinned down an API guarantee of that, and haven't experimented carefully with what happens if e.g. the text on the right is higher than the text on the left.
-
We then try to work out from that list of boxes which one is adjacent to the given offset — which one comes logically first (for _getMetricsFromDownstream) or last (for _getMetricsFromUpstream).
-
If the boxes are all LTR, or all RTL, one can do that. (Provided our assumption that they're given in left-to-right order holds.)
But if they're of different directions, I think there is not a way to do that reliably even in principle. For example if there's an LTR box to the left of an RTL box, either one could be the one that came first in the text.
Before #122480, our implementation effectively assumed RTL text. After #122480, it will work correctly for pure LTR text as well as for pure RTL text. But it will still not work for bidirectional text.
Like with #122477, in order to trigger this issue, one necessary ingredient is a grapheme cluster that's at least 3 code units long (specifically, it has to not be 1, 2, 4, 8, or another power of two code units long.) One way to do that is with a complex emoji, as in the #122477 recipe.