[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