[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