[Webkit-unassigned] [Bug 105451] Unable to place caret in RTL Override Characters by mouse click

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=105451





--- Comment #4 from Yi Shen <max.hong.shen at gmail.com>  2012-12-19 15:36:20 PST ---
Thanks for reviewing :]

The reason why I decide not to use nextLeafChild here is because I don't want to break the original code logic.  For example, let's say we have two line boxes, abc'\n'. If we use nextLeafChild to check whether current box is the last in the line, the line break box will get a chance to be checked by lineDirectionPointFitsInBox, right? If we use nextLeafChildIgnoringLineBreak, this won't happen.

In addition, I think the names of the prev/nextLeafChildIgnoringLineBreak are kind of confusing. It surprised me that they return 0 if there is a line break, instead of the leaf child before or after the line break.

The test select-line-break-with-opposite-directionality.html makes sense for me. In addition we can add a new test case as below (let me know if you want me to add it)

<li><div contenteditable contenteditable title="0 3">ab&#8238;c<br></div></li>

(In reply to comment #3)
> (From update of attachment 180204 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=180204&action=review
> 
> > Source/WebCore/rendering/RenderText.cpp:513
> > -    if (!box->nextLeafChildIgnoringLineBreak()) {
> > +    if (!box->nextLeafChildIgnoringLineBreak(false)) {
> 
> Just call nextLeafChild() instead. r- due to unnecessary changes to nextLeafChildIgnoringLineBreak and prevLeafChildIgnoringLineBreak.
> Did you make sure selection made by mouse drag on http://trac.webkit.org/browser/trunk/LayoutTests/editing/selection/select-line-break-with-opposite-directionality.html
> makes sense for all cases? (in addition to ones that are already tested).

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.