Bug 251313 – Incorrect text caret placement when RTL text starts with whitespace

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Ebrahim Byagowi]

This is a reduction of something that hits me on RTL websites and happens both in Safari 16.2 and tip of tree (5dc808f2):

Steps to reproduce:
Open
data:text/html;charset=utf8,<textarea%20dir=rtl%20style="font-size:%20500%">%20شسی%20شسی%20شسی%20شسی%20شسی%20شسی%20شسی%20شسی</textarea><script>document.querySelector('textarea').focus()</script>
(if caret didn't appear, click on the textarea, press Command+A, press the right arrow key button)

Expected:
The caret should go to right most position, start of the text in RTL.

Actual:
The caret goes after the first space, as can be seen in the attached screen cast.

[Ebrahim Byagowi]

Created attachment 464698 [details]
Caret

[zalan]

This is about incorrect hanging whitespace handling in RTL context. When the textarea's style is changed from "white-space: pre-wrap" to 'white-space: pre", it starts working fine.
Looking at the individual text run positions, the logically leading (visually trailing) whitespace on the first line is at -5px (this is logical x -5px, which essentially means whitespace is put outside the textarea).

[Radar WebKit Bug Importer]

<rdar://problem/104781435>

[zalan]

Created attachment 464742 [details]
Patch

[Ebrahim Byagowi]

> Patch

Just tested the patch and works as expected, awesome. Thanks 😊

[zalan]

(In reply to Ebrahim Byagowi from comment #5)
> > Patch
> 
> Just tested the patch and works as expected, awesome. Thanks 😊
Thank you so much! I decided to go with a more (RTL) focused approach though -will post a new patch soon (sorry about that, will use this patch as a more generic fix for pre-wrap vs. scrollable overflow under a different bug).

[zalan]

Created attachment 464751 [details]
Patch

[Ebrahim Byagowi]

Tested the new patch also and it works great, this use of C++ lambdas to contain variables needed to calculate one value is an interesting pattern I found in these fixes, hopefully I will use it somewhere I had access to a C++ codebase, and, thanks again 😊

[zalan]

(In reply to Ebrahim Byagowi from comment #8)
> Tested the new patch also and it works great, this use of C++ lambdas to
> contain variables needed to calculate one value is an interesting pattern I
> found in these fixes, hopefully I will use it somewhere I had access to a
> C++ codebase, and, thanks again 😊
I find it useful when I need to scope things for clarify (especially with this many lines of comment) but don't want to introduce a static helper function.

[zalan]

Created attachment 464805 [details]
Patch

[Ebrahim Byagowi]

Is that underline behind hanging space on svg/compositing/outermost-svg-with-border-padding.html worrisome? Apparently other browsers do that also,

data:text/html;charset=utf8,<u%20dir=rtl%20style="width:700px;display:block;font-size:%20500%;%20white-space:%20pre-wrap">%20%D8%B4%D8%B3%DB%8C%20%D8%B4%D8%B3%DB%8C%20%D8%B4%D8%B3%DB%8C%20%D8%B4%D8%B3%DB%8C%20%D8%B4%D8%B3%DB%8C%20%D8%B4%D8%B3%DB%8C%20%D8%B4%D8%B3%DB%8C%20%D8%B4%D8%B3%DB%8C</textarea><script>document.querySelector('textarea').focus()</u>

[Ebrahim Byagowi]

> Apparently other browsers do that also,

But it shouldn't happen beyond the element borders apparently, hmmm.

[zalan]

(In reply to Ebrahim Byagowi from comment #12)
> > Apparently other browsers do that also,
> 
> But it shouldn't happen beyond the element borders apparently, hmmm.
Chrome overhangs that trailing space too (see attached screenshot). going to take a look at that failing test case and land this patch shortly.

[zalan]

Created attachment 464830 [details]
overhanging trailing space

[zalan]

(this is with overflow: visible. the failing test has overflow auto/scroll. This is about whether the trailing hanging whitespace should contribute to scrollable or ink overflow -current behavior is ink, but with this patch we start contributing to scrollable overflow)

[zalan]

Created attachment 464842 [details]
[fast-cq]Patch

[Ebrahim Byagowi]

> Patch

Applied the patch on top of tip of tree (6c4c9810) and things look great, thanks 😊

[zalan]

(In reply to Ebrahim Byagowi from comment #17)
> > Patch
> 
> Applied the patch on top of tip of tree (6c4c9810) and things look great,
> thanks 😊
Thank you! Very much appreciated!

[EWS]

Committed 259868@main (f500d3412d93): <https://commits.webkit.org/259868@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 464842 [details].