[Webkit-unassigned] [Bug 27038] htmlediting.cpp needs more utility functions to fix the bug 26816

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=27038


Eric Seidel <eric at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #32402|review?                     |review-
               Flag|                            |




--- Comment #10 from Eric Seidel <eric at webkit.org>  2009-07-08 14:09:27 PDT ---
(From update of attachment 32402)
Style:
 63     short comparePoint(Node* refNode, int offset, ExceptionCode& ec) const;

I would have used early return:
 215     Element* enclosingCell = static_cast<Element*>(enclosingNodeOfType(p,
&isTableCell, true));
 216     // Since enclosingNodeOfType won't search beyond the highest root
editable node,
 217     // this code works even if the closest table cell was outside of the
root editable node.
 218     return enclosingCell ? enclosingCell : editableRootForPosition(p);

This could assert ASSERT(node->parentNode()):
 592 VisiblePosition visiblePositionBeforeNode(const Node* node)

Please add a FIXME: to the other createRange:
 612 PassRefPtr<Range> createRange(PassRefPtr<Document> document, const
VisiblePosition& start, const VisiblePosition& end, ExceptionCode& ec)

PassRefPtr seems the wrong type for these args:
 627 PassRefPtr<Range> extendRangeToWrappingNodes(PassRefPtr<Range>
rangeToExtend, PassRefPtr<Range> maximumRange, Node* rootNode)

Explicit visiblePosition construction is not needed here:
 1076     return visiblePositionBeforeNode(node) ==
VisiblePosition(selectedRange.startPosition())
(but it also doesn't hurt)

Style:
 65 Element* unsplittableElementForPosition(const Position& p);
 78 PassRefPtr<Range> createRange(PassRefPtr<Document> document, const
VisiblePosition& start, const VisiblePosition& end, ExceptionCode& ec);
 145 bool isNodeVisiblyContainedWithin(Node* node, const Range& selectedRange);

Looks fine.  Normally we don't add unused code, but I can see how this will
help make your future patches smaller.

Please re-submit a fixed version, and then we might wait to land this until the
other patch which depends on these changes is approved to.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.