Reviewed by Dave Harrison.
authormjs <mjs@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 9 May 2005 19:04:29 +0000 (19:04 +0000)
committermjs <mjs@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 9 May 2005 19:04:29 +0000 (19:04 +0000)
- remove remaining uses of upstream/downstream DoNotStayInBlock

        * khtml/editing/htmlediting.cpp:
        (khtml::CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessary):
        (khtml::ApplyStyleCommand::nodeFullySelected):
        (khtml::ApplyStyleCommand::nodeFullyUnselected):
        (khtml::DeleteSelectionCommand::insertPlaceholderForAncestorBlockContent):
        (khtml::InsertParagraphSeparatorCommand::doApply):
        (khtml::InsertParagraphSeparatorInQuotedContentCommand::doApply):
        (khtml::InsertTextCommand::insertSpace):
        (khtml::ReplaceSelectionCommand::doApply):

        * khtml/editing/visible_position.cpp:
        (khtml::enclosingBlockFlowElement): New helper function.
* khtml/editing/visible_position.h:

        * khtml/editing/visible_units.cpp:
        (khtml::inSameBlock): Check enclosing block flows instead of comparing
visible block starts. Two nested blocks may have the same visible start but
different visible ends, so the old check would give false positives.

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

WebCore/ChangeLog-2005-08-23
WebCore/khtml/editing/htmlediting.cpp
WebCore/khtml/editing/visible_position.cpp
WebCore/khtml/editing/visible_position.h
WebCore/khtml/editing/visible_units.cpp

index cd9914dba33dd84a5765252ef40cb0ed4004430c..4cdbeb8e3712f1ef7a3cb18aded2e390dfa232c9 100644 (file)
@@ -1,3 +1,28 @@
+2005-05-08  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Dave Harrison.
+
+       - remove remaining uses of upstream/downstream DoNotStayInBlock
+       
+        * khtml/editing/htmlediting.cpp:
+        (khtml::CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessary):
+        (khtml::ApplyStyleCommand::nodeFullySelected):
+        (khtml::ApplyStyleCommand::nodeFullyUnselected):
+        (khtml::DeleteSelectionCommand::insertPlaceholderForAncestorBlockContent):
+        (khtml::InsertParagraphSeparatorCommand::doApply):
+        (khtml::InsertParagraphSeparatorInQuotedContentCommand::doApply):
+        (khtml::InsertTextCommand::insertSpace):
+        (khtml::ReplaceSelectionCommand::doApply):
+       
+        * khtml/editing/visible_position.cpp:
+        (khtml::enclosingBlockFlowElement): New helper function.
+       * khtml/editing/visible_position.h:
+
+        * khtml/editing/visible_units.cpp:
+        (khtml::inSameBlock): Check enclosing block flows instead of comparing
+       visible block starts. Two nested blocks may have the same visible start but
+       different visible ends, so the old check would give false positives.
+
 2005-05-09  David Harrison  <harrison@apple.com>
 
         Add layout test for <rdar://problem/4110366>.
         * khtml/editing/htmlediting.cpp:
         (khtml::EditCommandPtr::setEndingSelection):
 
->>>>>>> 1.4172
 2005-05-02  Maciej Stachowiak  <mjs@apple.com>
 
         Reviewed by Vicki.
index aa50ead299c1e8c3c3caf61d67a8ade900251850..d14410cfac118a0b8b12e1a6fd41ccc3134b15e7 100644 (file)
@@ -70,7 +70,6 @@ using DOM::DocumentFragmentImpl;
 using DOM::DocumentImpl;
 using DOM::DOMString;
 using DOM::DOMStringImpl;
-using DOM::DoNotStayInBlock;
 using DOM::DoNotUpdateLayout;
 using DOM::EditingTextImpl;
 using DOM::ElementImpl;
@@ -1188,7 +1187,6 @@ void CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessary(const Posi
     VisiblePosition visibleParagraphEnd(endOfParagraph(visiblePos, IncludeLineBreak));
     Position paragraphStart = visibleParagraphStart.deepEquivalent().upstream(StayInBlock);
     Position paragraphEnd = visibleParagraphEnd.deepEquivalent().upstream(StayInBlock);
-    Position beforeParagraphStart = paragraphStart.upstream(DoNotStayInBlock);
     
     // Perform some checks to see if we need to perform work in this function.
     if (paragraphStart.node()->isBlockFlow()) {
@@ -1210,7 +1208,7 @@ void CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessary(const Posi
             return;
         }
     }
-    
+
     // Create the block to insert. Most times, this will be a shallow clone of the block containing
     // the start of the selection (the start block), except for two cases:
     //    1) When the start block is a body element.
@@ -1235,14 +1233,8 @@ void CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessary(const Posi
     else if (paragraphStart.node()->id() == ID_BR) {
         insertNodeAfter(newBlock, paragraphStart.node());
     }
-    else if (paragraphStart.node()->isBlockFlow()) {
-        insertNodeBefore(newBlock, paragraphStart.node());
-    }
-    else if (beforeParagraphStart.node()->enclosingBlockFlowElement()->id() != ID_BODY) {
-        insertNodeAfter(newBlock, beforeParagraphStart.node()->enclosingBlockFlowElement());
-    }
     else {
-        insertNodeAfter(newBlock, beforeParagraphStart.node());
+        insertNodeBefore(newBlock, paragraphStart.upstream(StayInBlock).node());
     }
 
     while (moveNode && !moveNode->isBlockFlow()) {
@@ -2135,7 +2127,7 @@ bool ApplyStyleCommand::nodeFullySelected(NodeImpl *node, const Position &start,
     ASSERT(node);
     ASSERT(node->isElementNode());
 
-    Position pos = Position(node, node->childNodeCount()).upstream(DoNotStayInBlock);
+    Position pos = Position(node, node->childNodeCount()).upstream(StayInBlock);
     return RangeImpl::compareBoundaryPoints(node, 0, start.node(), start.offset()) >= 0 &&
         RangeImpl::compareBoundaryPoints(pos, end) <= 0;
 }
@@ -2145,7 +2137,7 @@ bool ApplyStyleCommand::nodeFullyUnselected(NodeImpl *node, const Position &star
     ASSERT(node);
     ASSERT(node->isElementNode());
 
-    Position pos = Position(node, node->childNodeCount()).upstream(DoNotStayInBlock);
+    Position pos = Position(node, node->childNodeCount()).upstream(StayInBlock);
     bool isFullyBeforeStart = RangeImpl::compareBoundaryPoints(pos, start) < 0;
     bool isFullyAfterEnd = RangeImpl::compareBoundaryPoints(node, 0, end.node(), end.offset()) > 0;
 
@@ -2691,17 +2683,21 @@ void DeleteSelectionCommand::insertPlaceholderForAncestorBlockContent()
     // The checks below detect the case where the selection contains content in an ancestor block 
     // surrounded by child blocks.
     //
-    NodeImpl *upstreamBlock = m_upstreamStart.node()->enclosingBlockFlowElement();
-    NodeImpl *beforeUpstreamBlock = m_upstreamStart.upstream(DoNotStayInBlock).node()->enclosingBlockFlowElement();
-    
-    if (upstreamBlock != beforeUpstreamBlock && 
-        beforeUpstreamBlock->isAncestor(upstreamBlock) &&
-        upstreamBlock != m_upstreamStart.node()) {
-        NodeImpl *downstreamBlock = m_downstreamEnd.node()->enclosingBlockFlowElement();
-        NodeImpl *afterDownstreamBlock = m_downstreamEnd.downstream(DoNotStayInBlock).node()->enclosingBlockFlowElement();
+    VisiblePosition visibleStart(m_upstreamStart, VP_DEFAULT_AFFINITY);
+    VisiblePosition beforeStart = visibleStart.previous();
+    NodeImpl *startBlock = enclosingBlockFlowElement(visibleStart);
+    NodeImpl *beforeStartBlock = enclosingBlockFlowElement(beforeStart);
+    
+    if (!beforeStart.isNull() &&
+        !inSameBlock(visibleStart, beforeStart) &&
+        beforeStartBlock->isAncestor(startBlock) &&
+        startBlock != m_upstreamStart.node()) {
+
+        VisiblePosition visibleEnd(m_downstreamEnd, VP_DEFAULT_AFFINITY);
+        VisiblePosition afterEnd = visibleEnd.next();
         
-        if ((afterDownstreamBlock != downstreamBlock && afterDownstreamBlock != upstreamBlock) ||
-            (m_downstreamEnd == m_selectionToDelete.end() && isEndOfParagraph(VisiblePosition(m_downstreamEnd, VP_DEFAULT_AFFINITY)))) {
+        if ((!afterEnd.isNull() && !inSameBlock(afterEnd, visibleEnd) && !inSameBlock(afterEnd, visibleStart)) ||
+            (m_downstreamEnd == m_selectionToDelete.end() && isEndOfParagraph(visibleEnd))) {
             NodeImpl *block = createDefaultParagraphElement(document());
             insertNodeBefore(block, m_upstreamStart.node());
             addBlockPlaceholderIfNeeded(block);
@@ -3556,8 +3552,9 @@ void InsertParagraphSeparatorCommand::doApply()
     //---------------------------------------------------------------------
     // Handle case when position is in the first visible position in its block.
     // and similar case where upstream position is in another block.
-    bool upstreamInDifferentBlock = startBlock != pos.upstream(DoNotStayInBlock).node()->enclosingBlockFlowElement();
-    if (upstreamInDifferentBlock || isFirstInBlock) {
+    bool prevInDifferentBlock = !inSameBlock(visiblePos, visiblePos.previous());
+
+    if (prevInDifferentBlock || isFirstInBlock) {
         LOG(Editing, "insert paragraph separator: first in block case");
         pos = pos.downstream(StayInBlock);
         pos = positionOutsideContainingSpecialElement(pos);
@@ -3640,10 +3637,11 @@ void InsertParagraphSeparatorCommand::doApply()
         parent = child;
     }
 
-    // Insert a block placeholder in this case because we know that ther will be no content
-    // on the first line of the new block before the first block child of the new block.
-    // So, we need the placeholder to "hold the first line open".
-    if (startBlock != pos.downstream(DoNotStayInBlock).node()->enclosingBlockFlowElement())
+    // Insert a block placeholder if the next visible position is in a different paragraph,
+    // because we know that there will be no content on the first line of the new block 
+    // before the first block child. So, we need the placeholder to "hold the first line open".
+    VisiblePosition next = visiblePos.next();
+    if (!next.isNull() && !inSameBlock(visiblePos, next))
         appendBlockPlaceholder(blockToInsert);
 
     // Move the start node and the siblings of the start node.
@@ -3717,7 +3715,7 @@ void InsertParagraphSeparatorInQuotedContentCommand::doApply()
     EAffinity affinity = selection.startAffinity();
     if (selection.isRange()) {
         deleteSelection(false, false);
-        pos = endingSelection().start().upstream(DoNotStayInBlock);
+        pos = endingSelection().start().upstream(StayInBlock);
         affinity = endingSelection().startAffinity();
     }
     
@@ -3998,7 +3996,7 @@ void InsertTextCommand::insertSpace(TextImpl *textNode, unsigned long offset)
         // By checking the character at the downstream position, we can
         // check if there is a rendered WS at the caret
         Position pos(textNode, offset);
-        Position downstream = pos.downstream(DoNotStayInBlock);
+        Position downstream = pos.downstream(StayInBlock);
         if (downstream.offset() < (long)text.length() && isCollapsibleWhitespace(text[downstream.offset()]))
             count--; // leave this WS in
         if (count > 0)
@@ -4971,7 +4969,7 @@ void ReplaceSelectionCommand::doApply()
         else if (!insertionBlockIsRoot && isProbablyBlock(refNode) && isLastVisiblePositionInBlock(visiblePos)) {
             insertNodeAfterAndUpdateNodesInserted(refNode, insertionBlock);
         } else if (mergeStart && !isProbablyBlock(refNode)) {
-            Position pos = insertionPos.downstream(DoNotStayInBlock);
+            Position pos = visiblePos.next().deepEquivalent().downstream(StayInBlock);
             insertNodeAtAndUpdateNodesInserted(refNode, pos.node(), pos.offset());
         } else {
             insertNodeAtAndUpdateNodesInserted(refNode, insertionPos.node(), insertionPos.offset());
@@ -5028,30 +5026,32 @@ void ReplaceSelectionCommand::doApply()
         removeLinePlaceholderIfNeeded(linePlaceholder);
 
         if (!m_lastNodeInserted) {
-            lastPositionToSelect = endingSelection().end().downstream(DoNotStayInBlock);
+            lastPositionToSelect = endingSelection().end().downstream(StayInBlock);
         }
         else {
             bool insertParagraph = false;
+            VisiblePosition pos(insertionPos, VP_DEFAULT_AFFINITY);
+
             if (startBlock == endBlock && !isProbablyBlock(m_lastTopNodeInserted)) {
                 insertParagraph = true;
-            }
-            else {
+            } else {
                 // Handle end-of-document case.
                 document()->updateLayout();
-                VisiblePosition pos(Position(m_lastNodeInserted, m_lastNodeInserted->caretMaxOffset()), DOWNSTREAM);
                 if (isEndOfDocument(pos))
                     insertParagraph = true;
             }
             if (insertParagraph) {
                 setEndingSelection(insertionPos, DOWNSTREAM);
                 insertParagraphSeparator();
-                updateNodesInserted(endingSelection().end().downstream(DoNotStayInBlock).node());
+                VisiblePosition next = pos.next();
+
                 // Select up to the paragraph separator that was added.
-                lastPositionToSelect = endingSelection().end().downstream(DoNotStayInBlock);
-            } 
-            else {
+                lastPositionToSelect = next.deepEquivalent().downstream(StayInBlock);
+                updateNodesInserted(lastPositionToSelect.node());
+            else {
                 // Select up to the preexising paragraph separator.
-                lastPositionToSelect = Position(m_lastNodeInserted, m_lastNodeInserted->caretMaxOffset()).downstream(DoNotStayInBlock);
+                VisiblePosition next = pos.next();
+                lastPositionToSelect = next.deepEquivalent().downstream(StayInBlock);
             }
         }
     } 
index ae03b7f01e846c50ca7b33718e2301d08b06819e..2ed433c16af6d412ee55753ce3bb1538accffaa4 100644 (file)
@@ -449,6 +449,14 @@ void setAffinityUsingLinePosition(VisiblePosition &pos)
     }
 }
 
+DOM::NodeImpl *enclosingBlockFlowElement(const VisiblePosition &vp)
+{
+    if (vp.isNull())
+        return NULL;
+
+    return vp.position().node()->enclosingBlockFlowElement();
+}
+
 bool visiblePositionsOnDifferentLines(const VisiblePosition &pos1, const VisiblePosition &pos2)
 {
     if (pos1.isNull() || pos2.isNull())
index 67a95fedd2814ffa50eaf5a4ca49746189ff29a2..3abfebb7bef0e4ac7bb40de18be7ff75bcbe435f 100644 (file)
@@ -131,6 +131,8 @@ VisiblePosition endVisiblePosition(const DOM::RangeImpl *, EAffinity);
 
 void setAffinityUsingLinePosition(VisiblePosition &);
 
+DOM::NodeImpl *enclosingBlockFlowElement(const VisiblePosition &);
+
 bool visiblePositionsOnDifferentLines(const VisiblePosition &, const VisiblePosition &);
 bool visiblePositionsInDifferentBlocks(const VisiblePosition &, const VisiblePosition &);
 bool isFirstVisiblePositionOnLine(const VisiblePosition &);
index 04d84efb26dcdab4f9176aa02bb6b2afa2ab08ed..da29fc6630849f79c1969b735d40bc8d894d6b55 100644 (file)
@@ -741,7 +741,7 @@ VisiblePosition endOfBlock(const VisiblePosition &c, EIncludeLineBreak includeLi
 
 bool inSameBlock(const VisiblePosition &a, const VisiblePosition &b)
 {
-    return a.isNotNull() && startOfBlock(a) == startOfBlock(b);
+    return !a.isNull() && enclosingBlockFlowElement(a) == enclosingBlockFlowElement(b);
 }
 
 bool isStartOfBlock(const VisiblePosition &pos)