Bug 65356 – positionForPoint returns wrong VisiblePosition at bidi boundaries

• Status: RESOLVED
• Resolution: FIXED
• Component: UI Events

[Ryosuke Niwa]

Reproduction steps:
1. Paste "‎לשנת12" into a LTR block; e.g. http://simple-rte.rniwa.com/?editor=%3Cspan%20style%3D%22font-size%3A%205em%3Bpadding%3A%2020px%3B%22%3E%u200E%u05DC%u05E9%u05E0%u05EA12%3C/span%3E&dir=ltr&designmode=false&script=document.getElementById%28%27editor%27%29.focus%28%29%3B
2. Click on the right of ל.
3. Observe that the caret is placed on the left of 1.

Expected result:
Caret is placed on the right of ל.

[Ryosuke Niwa]

Created attachment 102328 [details]
test cases

Here's some test cases (offset values are obtained on ToT WebKit with my work-in-progress patch).

[Ryosuke Niwa]

Created attachment 102329 [details]
work in progress

[WebKit Review Bot]

Attachment 102329 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/rendering/InlineTextBox.cpp..." exit_code: 1

Source/WebCore/rendering/RenderText.cpp:498:  This { should be at the end of the previous line  [whitespace/braces] [4]
Source/WebCore/rendering/RenderText.cpp:526:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 2 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Early Warning System Bot]

Comment on attachment 102329 [details]
work in progress

Attachment 102329 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9262408

[WebKit Review Bot]

Comment on attachment 102329 [details]
work in progress

Attachment 102329 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9266371

[WebKit Review Bot]

Comment on attachment 102329 [details]
work in progress

Attachment 102329 [details] did not pass cr-mac-ews (chromium):
Output: http://queues.webkit.org/results/9266377

[WebKit Review Bot]

Comment on attachment 102329 [details]
work in progress

Attachment 102329 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/9267354

[WebKit Review Bot]

Comment on attachment 102329 [details]
work in progress

Attachment 102329 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/9265372

[Ryosuke Niwa]

Created attachment 102608 [details]
fixes the bug

[Early Warning System Bot]

Comment on attachment 102608 [details]
fixes the bug

Attachment 102608 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9289500

[Ryosuke Niwa]

Created attachment 102610 [details]
fixed qt build failure

[Jeremy Moskovich]

With the patch applied, when running the attached testcase when I click between the "ת" and the 2 in לשנת12 then the caret is placed to the right of the ל instead of between the ת and the 2 like I'd expect.

[Ryosuke Niwa]

(In reply to comment #12)
> With the patch applied, when running the attached testcase when I click between the "ת" and the 2 in לשנת12 then the caret is placed to the right of the ל instead of between the ת and the 2 like I'd expect.

Right.  That's because there's no caret position between ת and 2 in WebKit.  Offsets are assigned as:
(0)(4)1(5)2D(3)C(2)B(1)A(6)

[Jeremy Moskovich]

In TextEdit and FF I can place the caret between the ת and the 2 .

[Ryosuke Niwa]

(In reply to comment #14)
> In TextEdit and FF I can place the caret between the ת and the 2 .

You need to set the writing direction to LTR (Format > Text > Writing Direction) first.

[Ryosuke Niwa]

Ping reviewers.

[Dave Hyatt]

Comment on attachment 102610 [details]
fixed qt build failure

r=me

[Ryosuke Niwa]

(In reply to comment #17)
> (From update of attachment 102610 [details])
> r=me

Thanks for the review!  I'm very excited to land this patch.

[Ryosuke Niwa]

Committed r93369: <http://trac.webkit.org/changeset/93369>