[Webkit-unassigned] [Bug 27154] useless null-check statement in visible_units.cpp at logicalStartOfLine

[bugzilla-daemon at webkit.org]

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





--- Comment #7 from Xiaomei Ji <xji at chromium.org>  2009-07-14 10:26:03 PDT ---
Thanks for identifying and working on this bug!

logicalStartPositionForLine/logicalStartOfLine and 
logicalEndPositionForLine/logicalEndOfLine are introduced when fixing issue
24168
(RTL: Home/End key does not behave correctly in mixed bidi text in RTL document
).

Since Home/End key are logical keys, those logical operations are introduced,
while the original
startPositionForLine/startOfLine/endPositionForLine/endOfLine are preserved for
visual operations.

I think it is my bug having the following code in logicalStartOfLine (although
honorEditableBoundaryAtOrAfter wont be called twice).
if (visPos.isNull())
       return c.honorEditableBoundaryAtOrAfter(visPos);
return c.honorEditableBoundaryAtOrAfter(visPos);

Antonio Gomes's patch should fix it.

The structure of logicalStartOfLine was similar to that of startOfLine() which
have "if (visPos.isNotNull())". But during the testing, I did not find any
cases where the above condition is true, so, I removed it (see the last 4th
review feedback in comments #20 of issue 24168). I guess the above mistake was
introduced when I removed it (should remove the if block completely).

The structure of logicalEndOfLine is slightly different because I did find
cases where the returning position from logicalEndPositionForLine() need to be
adjusted (see comments in the code).

I am leaving the logicalStartPositionForLine and logicalStartOfLine (same for
end of line) as separate functions to follow the pattern of
startPositionForLine and startOfLine. I prefer to leave it as is (although
logicalStartOfLine() is pretty simple right now), but I do not have strong
opinion about it. Maybe mitz has better suggestions.

I did not replace any of the
startPositionForLine/startOfLine/endPositionForLine/endofLine simply because
they are visual operations and are used in several other places. I do not have
enough knowledge of whether those functionalities are correct or not and
whether their usages are correct or not.

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