[webkit-reviews] review granted: [Bug 27038] htmlediting.cpp needs more utility functions to fix the bug 26816 : [Attachment 32484] resubmission

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted Ryosuke Niwa
<ryosuke.niwa at gmail.com>'s request for review:
Bug 27038: htmlediting.cpp needs more utility functions to fix the bug 26816
https://bugs.webkit.org/show_bug.cgi?id=27038

Attachment 32484: resubmission
https://bugs.webkit.org/attachment.cgi?id=32484&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
Looks fine.

I think the intent of the old comments (that something needs fixing) is lost:
    // FIXME: This should ASSERT(node->parentNode())
 582	 ASSERT(node);
 583	 // FIXME: adding ASSERT(node->parentNode()); results in
editing/deleting/delete-ligature-001.html crashing
569584

No need to be const:
 595 VisiblePosition visiblePositionBeforeNode(const Node* node)

Generally we don't make RefCounted objects const, ever.  Because well...
they're refcounted. :)	You can't ref them w/ them being const.

I probably would have named the argument "range" and the returned value
"extendRange" but this is fine too:
 632 PassRefPtr<Range> extendRangeToWrappingNodes(PassRefPtr<Range>
rangeToExtend, const Range* maximumRange, const Node* rootNode)

Thanks.!