Bug 103067 – Keyboard caret movement in textarea with RTL Override Character can make tab unresponsive

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Yair Yogev]

Steps to repro:
1. Copy that 7th line from the source code of http://xkcd.com/1137/
(view-source:http://xkcd.com/1137/ from chrome) 
2. Type a line of text in the comments textarea in this very page, for example type "one two three" and press enter.
3. Paste that 7th line you copied before (it should appear in the beginning of the second line)
4. Use your mouse to select the "<title>xkcd: " prefix of that line you pasted and press the Delete key.
5. Now press Left
6. The tab is now unresponsive.

Tested using:
Google Chrome	25.0.1323.1 (Official Build 167142) dev-m
OS	Windows XP
WebKit	537.19 (@134111)
JavaScript	V8 3.15.1.2

and also using Windows Safari 5.1.7 (7534.57.2) with r131444 WebKit

[Jeremy Moskovich]

Able to reproduce on the following browsers:
* Chrome 25.0.1323.0 on OS X
* Safari 6.0.2 on OS X

Just to clarify step #4 - to select, click on the left side of the first line and drag all the way past the line to the right. "<title>xkcd: " will be selected.

[Yair Yogev]

Created attachment 175672 [details]
testcase

[Yi Shen]

Is anyone working on this issue? If not, I will work on it.

[Ryosuke Niwa]

Go for it!

[Yi Shen]

Created attachment 177801 [details]
first try

A quick fix for this crash issue.

[Ryosuke Niwa]

Comment on attachment 177801 [details]
first try

View in context: https://bugs.webkit.org/attachment.cgi?id=177801&action=review

> Source/WebCore/editing/VisiblePosition.cpp:149
>                  InlineBox* prevBox = box->prevLeafChild();
> -                if (!prevBox) {
> +                if (!prevBox || prevBox->isLineBreak()) {

I think what you want to do here is to call prevLeafChildIgnoringLineBreak() in the previous line.
Also, you probably need to do the same thing in leftVisuallyDistinctCandidate.

[Ryosuke Niwa]

Comment on attachment 177801 [details]
first try

r- because it’s missing a fix in leftVisuallyDistinctCandidate.

[Yair Yogev]

did you mean rightVisuallyDistinctCandidate ?

[Ryosuke Niwa]

(In reply to comment #8)
> did you mean rightVisuallyDistinctCandidate ?

Oops, yeah, that’s what I meant.

[Yi Shen]

Created attachment 177825 [details]
updated base on Niwa's review

Thanks for the review, niwa :)

[Ryosuke Niwa]

Comment on attachment 177825 [details]
updated base on Niwa's review

View in context: https://bugs.webkit.org/attachment.cgi?id=177825&action=review

> Source/WebCore/editing/VisiblePosition.cpp:148
> -                InlineBox* prevBox = box->prevLeafChild();
> +                InlineBox* prevBox = box->prevLeafChildIgnoringLineBreak();

Looks much better!

> LayoutTests/editing/selection/move-by-character-crash-test-textarea.html:11
> +    var test = document.getElementById("test_move_by_character");

Please use camelCase.

[Yi Shen]

Created attachment 178641 [details]
fix test

[WebKit Review Bot]

Comment on attachment 178641 [details]
fix test

Rejecting attachment 178641 [details] from commit-queue.

[e-mail] does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.

[WebKit Review Bot]

Comment on attachment 178641 [details]
fix test

Clearing flags on attachment: 178641

Committed r137213: <http://trac.webkit.org/changeset/137213>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.