[webkit-reviews] review denied: [Bug 20092] Spelling markers positioned incorrectly in RTL text : [Attachment 23761] Updated patch.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Sep 24 15:07:40 PDT 2008
mitz at webkit.org has denied Jeremy Moskovich <playmobil at google.com>'s request
for review:
Bug 20092: Spelling markers positioned incorrectly in RTL text
https://bugs.webkit.org/show_bug.cgi?id=20092
Attachment 23761: Updated patch.
https://bugs.webkit.org/attachment.cgi?id=23761&action=edit
------- Additional Comments from mitz at webkit.org
Looks great! I did notice a couple of issues, though, now that the code is
cleaned up:
+ endPosition = min(endPosition, startPosition + m_truncation);
That looks wrong, because m_truncation is relative to m_start, not
startPosition.
+ IntPoint startPoint = IntPoint(tx, ty);
The current WebKit style for writing this is:
IntPoint startPoint(tx, ty);
However, the y coordinate is wrong. Currenrly, it is ty + selectionTop(), which
is also wrong since earlier m_y is added to ty. This means that grammar "tool
tips" only work in the first line of text in a block. What you really want
there is the passed-in ty + selectionTop().
-
+
Extra white space.
Sorry about not catching the above earlier. I am going to r- because the
endPosition issue looks like a regression.
More information about the webkit-reviews
mailing list