Changeset 29456 in webkit

Timestamp:

Jan 13, 2008 5:35:00 PM (16 years ago)

Author:

eric@webkit.org

Message:

Reviewed by darin.

Range.insertNode does not update endContainer endIndex correctly
in the case where it had to split a text node.
​http://bugs.webkit.org/show_bug.cgi?id=16765

Darin pointed out during review that we still don't handle the dynamic
range case (where the dom tree changes not using range methods)
Thus this code will get ripped out when we add that. The test cases
are still valid and useful however, so I'm landing this as-is.

Tests: fast/dom/Range/range-insertNode-separate-endContainer.html

fast/dom/Range/range-insertNode-splittext.html

• dom/Range.cpp: (WebCore::Range::insertNode): handle the splitText case correctly.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/dom/Range/range-insertNode-separate-endContainer-expected.txt (added)
• LayoutTests/fast/dom/Range/range-insertNode-separate-endContainer.html (added)
• LayoutTests/fast/dom/Range/range-insertNode-splittext-expected.txt (added)
• LayoutTests/fast/dom/Range/range-insertNode-splittext.html (added)
• LayoutTests/fast/dom/Range/resources/range-insertNode-separate-endContainer.js (added)
• LayoutTests/fast/dom/Range/resources/range-insertNode-splittext.js (added)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/dom/Range.cpp (modified) (4 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2008-01-13  Eric Seidel  <eric@webkit.org>
+
+        Reviewed by darin.
+        
+        range-insertNode-separate-endContainer validates proper range behavior
+        and does not change with this patch.
+        range-insertNode-splittext demonstrates the issue fixed by this patch
+        http://bugs.webkit.org/show_bug.cgi?id=16765
+
+        * fast/dom/Range/range-insertNode-separate-endContainer-expected.txt: Added.
+        * fast/dom/Range/range-insertNode-separate-endContainer.html: Added.
+        * fast/dom/Range/range-insertNode-splittext-expected.txt: Added.
+        * fast/dom/Range/range-insertNode-splittext.html: Added.
+        * fast/dom/Range/resources/range-insertNode-separate-endContainer.js: Added.
+        * fast/dom/Range/resources/range-insertNode-splittext.js: Added.
+
 2008-01-13  Dan Bernstein  <mitz@apple.com>
 

trunk/WebCore/ChangeLog

@@ +1 @@
+2008-01-13  Eric Seidel  <eric@webkit.org>
+
+        Reviewed by darin.
+
+        Range.insertNode does not update endContainer endIndex correctly
+        in the case where it had to split a text node.
+        http://bugs.webkit.org/show_bug.cgi?id=16765
+        
+        Darin pointed out during review that we still don't handle the dynamic
+        range case (where the dom tree changes not using range methods)
+        Thus this code will get ripped out when we add that.  The test cases
+        are still valid and useful however, so I'm landing this as-is.
+
+        Tests: fast/dom/Range/range-insertNode-separate-endContainer.html
+               fast/dom/Range/range-insertNode-splittext.html
+
+        * dom/Range.cpp:
+        (WebCore::Range::insertNode): handle the splitText case correctly.
+
 2008-01-13  Darin Adler  <darin@apple.com>
 

trunk/WebCore/dom/Range.cpp

@@ -980 +980 @@
 void Range::insertNode(PassRefPtr<Node> newNode, ExceptionCode& ec)
 {
+    ec = 0;
+
     if (m_detached) {
         ec = INVALID_STATE_ERR;
@@ -1031 +1033 @@
             }
         }
-    }
-    else {
+    } else {
         if (!checkAgainst->childTypeAllowed(newNode->nodeType())) {
             ec = HIERARCHY_REQUEST_ERR;
@@ -1047 +1048 @@
 
     // INVALID_NODE_TYPE_ERR: Raised if newNode is an Attr, Entity, Notation, or Document node.
-    if( newNode->nodeType() == Node::ATTRIBUTE_NODE ||
+    if (newNode->nodeType() == Node::ATTRIBUTE_NODE ||
         newNode->nodeType() == Node::ENTITY_NODE ||
         newNode->nodeType() == Node::NOTATION_NODE ||
@@ -1055 +1056 @@
     }
 
-    unsigned offsetDelta = 0;
-    if (m_startContainer == m_endContainer) {
-        bool isFragment = newNode->nodeType() == Node::DOCUMENT_FRAGMENT_NODE;
-        offsetDelta = isFragment ? newNode->childNodeCount() : 1;
-    }
-    
-    if(m_startContainer->nodeType() == Node::TEXT_NODE ||
-       m_startContainer->nodeType() == Node::CDATA_SECTION_NODE) {
-        ec = 0;
+    unsigned endOffsetDelta = 0;
+    if (m_startContainer->nodeType() == Node::TEXT_NODE ||
+        m_startContainer->nodeType() == Node::CDATA_SECTION_NODE) {
         RefPtr<Text> newText = static_cast<Text*>(m_startContainer.get())->splitText(m_startOffset, ec);
         if (ec)
             return;
+
+        if (m_startContainer == m_endContainer)
+            endOffsetDelta = -m_startOffset;
+
         m_startContainer->parentNode()->insertBefore(newNode, newText.get(), ec);
+        if (ec)
+            return;
+        m_endContainer = newText;
     } else {
+        if (m_startContainer == m_endContainer) {
+            bool isFragment = newNode->nodeType() == Node::DOCUMENT_FRAGMENT_NODE;
+            endOffsetDelta = isFragment ? newNode->childNodeCount() : 1;
+        }
+
         m_startContainer->insertBefore(newNode, m_startContainer->childNode(m_startOffset), ec);
-    }
-    
-    m_endOffset += offsetDelta;
+        if (ec)
+            return;
+    }
+    m_endOffset += endOffsetDelta;
 }