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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Aug 25 10:41:30 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 38519: patch w/ layout test
https://bugs.webkit.org/attachment.cgi?id=38519&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
 else {
 1819	      return;
 1820	  }
no {}

No "return;" needed.

1     Position s = rangeCompliantEquivalent(start);
 1822	  Position e = rangeCompliantEquivalent(end);
 1823	  setStart(s.node(), s.deprecatedEditingOffset(), ec);
 1824	  setEnd(e.node(), e.deprecatedEditingOffset(), ec);

That should be:

setStart(s.containerNode(), s.computeOffsetInContainerNode());
setEnd(e.containerNode(), e.computeOffsetInContainerNode());

no rangeCompliantEquivalent calls are needed if you use containerNode and
computeOffsetInContainerNode

deprecatedEditingOffset() is basically always the wrong call to use. In general
any call with "deprecated" in its name can be assumed to be. :)  The confusion
here is that soooo much code still uses deprecatedEditingOffset() :(


More information about the webkit-reviews mailing list