[webkit-reviews] review denied: [Bug 27632] Expose text segmentation through JavaScript : [Attachment 38574] patch w/ layout test

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Aug 25 16:49:26 PDT 2009


Eric Seidel <eric at webkit.org> has denied Xiaomei Ji <xji at chromium.org>'s
request for review:
Bug 27632: Expose text segmentation through JavaScript
https://bugs.webkit.org/show_bug.cgi?id=27632

Attachment 38574: patch w/ layout test
https://bugs.webkit.org/attachment.cgi?id=38574&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
Ok, in general this looks fine.

+    VisiblePosition start = VisiblePosition(this->startPosition());
+    VisiblePosition end = VisiblePosition(this->endPosition());

Should just be:
+    VisiblePosition start(startPosition());
+    VisiblePosition end(endPosition());

I don't think this comment is really needed, but it's OK to leave too. :)
+    // Please refer to https://bugs.webkit.org/show_bug.cgi?id=27632 comment
#5 for the spec.

This comment doesn't add anything:
+	 // Range expand.

If you were a committer, I would just r+ and you could fix those nits on
landing.  As is, it would be best if you could post one final patch. :)

Thanks!  Sorry I missed these on the first round.


More information about the webkit-reviews mailing list