2011-03-10 Ryosuke Niwa <rniwa@webkit.org>
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 Mar 2011 00:52:53 +0000 (00:52 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 Mar 2011 00:52:53 +0000 (00:52 +0000)
        Reviewed by Tony Chang.

        startOfBlock and endOfBlock may return a position inside hr
        https://bugs.webkit.org/show_bug.cgi?id=56025

        Replaced calls to enclosingBlockFlowElement in startOfBlock and endOfBlock by
        calls to enclosingBlock. Added EditingBoundaryCrossingRule to the argument lists
        of startOfBlock, endOfBlock, and enclosingBlock. Also replaced the last boolean
        argument variable of enclosingNodeOfType by EditingBoundaryCrossingRule.

        Also replaced calls to enclosingBlockFlowElement in inSameBlock by calls to
        enclosingBlock to be consitent with startOfBlock and endOfBlock.

        This patch also replaced calls to deprecatedNode in startOfBlock, endOfBlock,
        and inSameBlock by calls to containerNode because the enclosing block of a position
        should never be before or after the position.

        No tests are added because this change only affects WebCore internally.

        * editing/DeleteSelectionCommand.cpp:
        (WebCore::DeleteSelectionCommand::initializePositionData): Calls enclosingNodeOfType.
        Pass CanCrossEditingBoundary instead of false.
        * editing/htmlediting.cpp:
        (WebCore::unsplittableElementForPosition): Ditto.
        (WebCore::enclosingBlock): Takes EditingBoundaryCrossingRule and passes it to
        enclosingNodeOfType.
        (WebCore::enclosingNodeOfType): Takes EditingBoundaryCrossingRule instead of boolean.
        Fixed a bug that it stops walking the tree when it reached the root editable node
        even when the editing boundary crossing rule is CanCrossEditingBoundary.
        * editing/htmlediting.h: Prototype changes.
        * editing/visible_units.cpp:
        (WebCore::startOfBlock): Calls enclosingBlock instead of enclosingBlockFlowElement.
        Also added an early exit when there's no enclosing block.
        (WebCore::endOfBlock): Ditto. The early exist in this case prevents crash in
        lastPositionInNode.
        (WebCore::inSameBlock): Calls enclosingBlock instead of enclosingBlockFlowElement.
        (WebCore::isStartOfBlock): Calls startOfBlock with CanCrossEditingBoundary because
        we don't care where the start of block is when we're comparing against the given position.
        (WebCore::isEndOfBlock): Ditto.
        * editing/visible_units.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@80793 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/editing/DeleteSelectionCommand.cpp
Source/WebCore/editing/htmlediting.cpp
Source/WebCore/editing/htmlediting.h
Source/WebCore/editing/visible_units.cpp
Source/WebCore/editing/visible_units.h

index 35df63e207bc04bda37df898ca4b6f247deb730c..6193fd763064fc8fc1e16503487e80f5788f5f57 100644 (file)
@@ -1,3 +1,46 @@
+2011-03-10  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Reviewed by Tony Chang.
+
+        startOfBlock and endOfBlock may return a position inside hr
+        https://bugs.webkit.org/show_bug.cgi?id=56025
+
+        Replaced calls to enclosingBlockFlowElement in startOfBlock and endOfBlock by
+        calls to enclosingBlock. Added EditingBoundaryCrossingRule to the argument lists
+        of startOfBlock, endOfBlock, and enclosingBlock. Also replaced the last boolean
+        argument variable of enclosingNodeOfType by EditingBoundaryCrossingRule.
+
+        Also replaced calls to enclosingBlockFlowElement in inSameBlock by calls to
+        enclosingBlock to be consitent with startOfBlock and endOfBlock.
+
+        This patch also replaced calls to deprecatedNode in startOfBlock, endOfBlock,
+        and inSameBlock by calls to containerNode because the enclosing block of a position
+        should never be before or after the position.
+
+        No tests are added because this change only affects WebCore internally.
+
+        * editing/DeleteSelectionCommand.cpp:
+        (WebCore::DeleteSelectionCommand::initializePositionData): Calls enclosingNodeOfType.
+        Pass CanCrossEditingBoundary instead of false.
+        * editing/htmlediting.cpp:
+        (WebCore::unsplittableElementForPosition): Ditto.
+        (WebCore::enclosingBlock): Takes EditingBoundaryCrossingRule and passes it to
+        enclosingNodeOfType.
+        (WebCore::enclosingNodeOfType): Takes EditingBoundaryCrossingRule instead of boolean.
+        Fixed a bug that it stops walking the tree when it reached the root editable node
+        even when the editing boundary crossing rule is CanCrossEditingBoundary.
+        * editing/htmlediting.h: Prototype changes.
+        * editing/visible_units.cpp:
+        (WebCore::startOfBlock): Calls enclosingBlock instead of enclosingBlockFlowElement.
+        Also added an early exit when there's no enclosing block.
+        (WebCore::endOfBlock): Ditto. The early exist in this case prevents crash in
+        lastPositionInNode.
+        (WebCore::inSameBlock): Calls enclosingBlock instead of enclosingBlockFlowElement.
+        (WebCore::isStartOfBlock): Calls startOfBlock with CanCrossEditingBoundary because
+        we don't care where the start of block is when we're comparing against the given position.
+        (WebCore::isEndOfBlock): Ditto.
+        * editing/visible_units.h:
+
 2011-03-10  Alexey Proskuryakov  <ap@apple.com>
 
         Reviewed by Dan Bernstein.
index 897c3058ccdc7746556324e2ff9a411b5148f5da..065ec17832dcb16d7bf3035ddac9b94c6d773006 100644 (file)
@@ -188,8 +188,8 @@ void DeleteSelectionCommand::initializePositionData()
     // Don't move content out of a table cell.
     // If the cell is non-editable, enclosingNodeOfType won't return it by default, so
     // tell that function that we don't care if it returns non-editable nodes.
-    Node* startCell = enclosingNodeOfType(m_upstreamStart, &isTableCell, false);
-    Node* endCell = enclosingNodeOfType(m_downstreamEnd, &isTableCell, false);
+    Node* startCell = enclosingNodeOfType(m_upstreamStart, &isTableCell, CanCrossEditingBoundary);
+    Node* endCell = enclosingNodeOfType(m_downstreamEnd, &isTableCell, CanCrossEditingBoundary);
     // FIXME: This isn't right.  A borderless table with two rows and a single column would appear as two paragraphs.
     if (endCell && endCell != startCell)
         m_mergeBlocksAfterDelete = false;
@@ -262,8 +262,8 @@ void DeleteSelectionCommand::initializePositionData()
     // like the one below, since editing functions should obviously accept editing positions.
     // FIXME: Passing false to enclosingNodeOfType tells it that it's OK to return a non-editable
     // node.  This was done to match existing behavior, but it seems wrong.
-    m_startBlock = enclosingNodeOfType(m_downstreamStart.parentAnchoredEquivalent(), &isBlock, false);
-    m_endBlock = enclosingNodeOfType(m_upstreamEnd.parentAnchoredEquivalent(), &isBlock, false);
+    m_startBlock = enclosingNodeOfType(m_downstreamStart.parentAnchoredEquivalent(), &isBlock, CanCrossEditingBoundary);
+    m_endBlock = enclosingNodeOfType(m_upstreamEnd.parentAnchoredEquivalent(), &isBlock, CanCrossEditingBoundary);
 }
 
 void DeleteSelectionCommand::saveTypingStyleState()
index a7676f8b6270627e9c2e86fd27eb1eb6b3a6cb34..31cfa83dd03f18c5625ce2887b216291e97f21f3 100644 (file)
@@ -221,7 +221,7 @@ Element* unsplittableElementForPosition(const Position& p)
 {
     // Since enclosingNodeOfType won't search beyond the highest root editable node,
     // this code works even if the closest table cell was outside of the root editable node.
-    Element* enclosingCell = static_cast<Element*>(enclosingNodeOfType(p, &isTableCell, true));
+    Element* enclosingCell = static_cast<Element*>(enclosingNodeOfType(p, &isTableCell));
     if (enclosingCell)
         return enclosingCell;
 
@@ -327,9 +327,9 @@ bool isBlock(const Node* node)
 // FIXME: Pass a position to this function.  The enclosing block of [table, x] for example, should be the 
 // block that contains the table and not the table, and this function should be the only one responsible for 
 // knowing about these kinds of special cases.
-Node* enclosingBlock(Node* node)
+Node* enclosingBlock(Node* node, EditingBoundaryCrossingRule rule)
 {
-    return static_cast<Element*>(enclosingNodeOfType(firstPositionInOrBeforeNode(node), isBlock));
+    return static_cast<Element*>(enclosingNodeOfType(firstPositionInOrBeforeNode(node), isBlock, rule));
 }
 
 // This method is used to create positions in the DOM. It returns the maximum valid offset
@@ -603,8 +603,10 @@ Node* enclosingNodeWithTag(const Position& p, const QualifiedName& tagName)
     return 0;
 }
 
-Node* enclosingNodeOfType(const Position& p, bool (*nodeIsOfType)(const Node*), bool onlyReturnEditableNodes)
+Node* enclosingNodeOfType(const Position& p, bool (*nodeIsOfType)(const Node*), EditingBoundaryCrossingRule rule)
 {
+    // FIXME: support CanSkipCrossEditingBoundary
+    ASSERT(rule == CanCrossEditingBoundary || rule == CannotCrossEditingBoundary);
     if (p.isNull())
         return 0;
         
@@ -612,11 +614,11 @@ Node* enclosingNodeOfType(const Position& p, bool (*nodeIsOfType)(const Node*),
     for (Node* n = p.deprecatedNode(); n; n = n->parentNode()) {
         // Don't return a non-editable node if the input position was editable, since
         // the callers from editing will no doubt want to perform editing inside the returned node.
-        if (root && !n->isContentEditable() && onlyReturnEditableNodes)
+        if (root && !n->isContentEditable() && rule == CannotCrossEditingBoundary)
             continue;
         if ((*nodeIsOfType)(n))
             return n;
-        if (n == root)
+        if (n == root && rule == CannotCrossEditingBoundary)
             return 0;
     }
     
index b71e8791bfd483aacae7e05d315e6ccc3c8cfa86..53f3eacbfa900ddf2f82890064e116a993aed329 100644 (file)
@@ -26,6 +26,7 @@
 #ifndef htmlediting_h
 #define htmlediting_h
 
+#include "EditingBoundary.h"
 #include "ExceptionCode.h"
 #include "HTMLNames.h"
 #include "Position.h"
@@ -57,12 +58,12 @@ Node* highestEditableRoot(const Position&);
 Node* highestEnclosingNodeOfType(const Position&, bool (*nodeIsOfType)(const Node*));
 Node* lowestEditableAncestor(Node*);   
 
-Node* enclosingBlock(Node*);
+Node* enclosingBlock(Node*, EditingBoundaryCrossingRule = CannotCrossEditingBoundary);
 Node* enclosingTableCell(const Position&);
 Node* enclosingEmptyListItem(const VisiblePosition&);
 Node* enclosingAnchorElement(const Position&);
 Node* enclosingNodeWithTag(const Position&, const QualifiedName&);
-Node* enclosingNodeOfType(const Position&, bool (*nodeIsOfType)(const Node*), bool onlyReturnEditableNodes = true);
+Node* enclosingNodeOfType(const Position&, bool (*nodeIsOfType)(const Node*), EditingBoundaryCrossingRule = CannotCrossEditingBoundary);
 
 Node* tabSpanNode(const Node*);
 Node* nearestMailBlockquote(const Node*);
@@ -237,7 +238,7 @@ inline bool isWhitespace(UChar c)
 {
     return c == noBreakSpace || c == ' ' || c == '\n' || c == '\t';
 }
-String stringWithRebalancedWhitespace(const String&, bool, bool);
+String stringWithRebalancedWhitespace(const String&, bool startIsStartOfParagraph, bool endIsEndOfParagraph);
 const String& nonBreakingSpaceString();
 
 }
index 43ce264f24f97ba8bc77a5b74a218ddd125f3b6c..5e35bd0a663b29171b3eb4407dcc54a3f13a2a52 100644 (file)
@@ -934,41 +934,37 @@ VisiblePosition nextParagraphPosition(const VisiblePosition& p, int x)
 
 // ---------
 
-VisiblePosition startOfBlock(const VisiblePosition &c)
+VisiblePosition startOfBlock(const VisiblePosition& visiblePosition, EditingBoundaryCrossingRule rule)
 {
-    Position p = c.deepEquivalent();
-    Node* startNode = p.deprecatedNode();
-    if (!startNode)
+    Position position = visiblePosition.deepEquivalent();
+    Node* startBlock;
+    if (!position.containerNode() || !(startBlock = enclosingBlock(position.containerNode(), rule)))
         return VisiblePosition();
-    return VisiblePosition(firstPositionInNode(startNode->enclosingBlockFlowElement()), DOWNSTREAM);
+    return firstPositionInNode(startBlock);
 }
 
-VisiblePosition endOfBlock(const VisiblePosition &c)
+VisiblePosition endOfBlock(const VisiblePosition& visiblePosition, EditingBoundaryCrossingRule rule)
 {
-    Position p = c.deepEquivalent();
-
-    Node* startNode = p.deprecatedNode();
-    if (!startNode)
+    Position position = visiblePosition.deepEquivalent();
+    Node* endBlock;
+    if (!position.containerNode() || !(endBlock = enclosingBlock(position.containerNode(), rule)))
         return VisiblePosition();
-
-    Node *startBlock = startNode->enclosingBlockFlowElement();
-    
-    return VisiblePosition(lastPositionInNode(startBlock), VP_DEFAULT_AFFINITY);   
+    return lastPositionInNode(endBlock);
 }
 
 bool inSameBlock(const VisiblePosition &a, const VisiblePosition &b)
 {
-    return !a.isNull() && enclosingBlockFlowElement(a) == enclosingBlockFlowElement(b);
+    return !a.isNull() && enclosingBlock(a.deepEquivalent().containerNode()) == enclosingBlock(b.deepEquivalent().containerNode());
 }
 
 bool isStartOfBlock(const VisiblePosition &pos)
 {
-    return pos.isNotNull() && pos == startOfBlock(pos);
+    return pos.isNotNull() && pos == startOfBlock(pos, CanCrossEditingBoundary);
 }
 
 bool isEndOfBlock(const VisiblePosition &pos)
 {
-    return pos.isNotNull() && pos == endOfBlock(pos);
+    return pos.isNotNull() && pos == endOfBlock(pos, CanCrossEditingBoundary);
 }
 
 // ---------
index 5717e2931400e988804ca35d9a2516701a12431d..c2f9c5e609cd2e6e23bc2ec2948615ef7c8c25ac 100644 (file)
@@ -73,8 +73,8 @@ bool isEndOfParagraph(const VisiblePosition &, EditingBoundaryCrossingRule = Can
 bool inSameParagraph(const VisiblePosition &, const VisiblePosition &, EditingBoundaryCrossingRule = CannotCrossEditingBoundary);
 
 // blocks (true paragraphs; line break elements don't break blocks)
-VisiblePosition startOfBlock(const VisiblePosition &);
-VisiblePosition endOfBlock(const VisiblePosition &);
+VisiblePosition startOfBlock(const VisiblePosition &, EditingBoundaryCrossingRule = CannotCrossEditingBoundary);
+VisiblePosition endOfBlock(const VisiblePosition &, EditingBoundaryCrossingRule = CannotCrossEditingBoundary);
 bool inSameBlock(const VisiblePosition &, const VisiblePosition &);
 bool isStartOfBlock(const VisiblePosition &);
 bool isEndOfBlock(const VisiblePosition &);