Reviewed by Darin.

        http://bugs.webkit.org/show_bug.cgi?id=15892
        DOM Range operations are not implemented for ProcessingInstruction nodes

        Test: fast/dom/Range/range-processing-instructions.html

        * dom/Range.cpp:
        (WebCore::Range::processContents): Implemented ProcessingInstruction cases.
        (WebCore::Range::checkNodeWOffset): Removed a FIXME - yes, I think that we are supposed
        to use ProcessingInstruction.data.

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index a13fbfc..a6c05fe 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2007-11-10  Alexey Proskuryakov  <ap@nypop.com>
+
+        Reviewed by Darin.
+
+        http://bugs.webkit.org/show_bug.cgi?id=15892
+        DOM Range operations are not implemented for ProcessingInstruction nodes
+
+        * fast/dom/Range/range-processing-instructions-expected.txt: Added.
+        * fast/dom/Range/range-processing-instructions.html: Added.
+
 2007-11-09  Tristan O'Tierney  <tristan@apple.com>
 
         Reviewed by Sam Weinig.

diff --git a/LayoutTests/fast/dom/Range/range-processing-instructions-expected.txt b/LayoutTests/fast/dom/Range/range-processing-instructions-expected.txt
new file mode 100644 (file)
index 0000000..a730dc0
--- /dev/null
+++ b/LayoutTests/fast/dom/Range/range-processing-instructions-expected.txt
@@ -0,0 +1,4 @@
+Test that Range manipulations work with ProcessingInstruction nodes.
+Test 1: SUCCESS
+Test 2: SUCCESS
+Test 3: SUCCESS

diff --git a/LayoutTests/fast/dom/Range/range-processing-instructions.html b/LayoutTests/fast/dom/Range/range-processing-instructions.html
new file mode 100644 (file)
index 0000000..013619e
--- /dev/null
+++ b/LayoutTests/fast/dom/Range/range-processing-instructions.html
@@ -0,0 +1,68 @@
+<div>Test that Range manipulations work with ProcessingInstruction nodes.</div>
+<script>
+  if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+
+  function dumpRange(range) {
+      if (range.startContainer == range.endContainer)
+          return range.startContainer.nodeName + " " + range.startOffset + " - " + range.endOffset;
+      else
+          return range.startContainer.nodeName + " " + range.startOffset + " - " + range.endContainer.nodeName + " " + range.endOffset;
+  }
+
+  var docString = "<foo><" + "?pi SUCC_FAILURE_ESS?><" + "?pi SUCC_FAILURE_ESS?></foo>";
+
+  try {
+    var doc = (new DOMParser).parseFromString(docString, "application/xml");
+    var foo = doc.getElementsByTagName("foo")[0];
+
+    var range = doc.createRange();
+    range.setStart(foo.firstChild, 4);
+    range.setEnd(foo.firstChild, 13);
+    range.deleteContents();
+
+    document.write("Test 1: " + foo.firstChild.data);
+  } catch (ex) {
+    document.write("Test 1: " + ex);
+  }
+  
+  document.write("<br>");
+  
+  try {
+    var doc = (new DOMParser).parseFromString(docString, "application/xml");
+    var foo = doc.getElementsByTagName("foo")[0];
+
+    var range = doc.createRange();
+    range.setStart(foo.firstChild, 4);
+    range.setEnd(foo.firstChild.nextSibling, 13);
+    range.deleteContents();
+
+    document.write("Test 2: " + foo.firstChild.data + foo.firstChild.nextSibling.data);
+  } catch (ex) {
+    document.write("Test 2: " + ex);
+  }
+
+  document.write("<br>");
+
+  try {
+    var doc = (new DOMParser).parseFromString(docString, "application/xml");
+    var foo = doc.getElementsByTagName("foo")[0];
+
+    var range = doc.createRange();
+    range.setStart(foo, 0);
+    range.setEnd(foo.firstChild, 2);
+    var data1 = range.cloneContents().firstChild.data;
+
+    range.setStart(foo.firstChild, 2);
+    range.setEnd(foo.firstChild, 4);
+    var data2 = range.cloneContents().firstChild.data;
+
+    range.setStart(foo.firstChild.nextSibling, 13);
+    range.setEnd(foo, 2);
+    var data3 = range.cloneContents().firstChild.data;
+
+    document.write("Test 3: " + data1 + data2 + data3);
+  } catch (ex) {
+    document.write("Test 3: " + ex);
+  }
+</script>

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 9ac2aea..2ff6dca 100644 (file)
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,17 @@
+2007-11-10  Alexey Proskuryakov  <ap@nypop.com>
+
+        Reviewed by Darin.
+
+        http://bugs.webkit.org/show_bug.cgi?id=15892
+        DOM Range operations are not implemented for ProcessingInstruction nodes
+
+        Test: fast/dom/Range/range-processing-instructions.html
+
+        * dom/Range.cpp:
+        (WebCore::Range::processContents): Implemented ProcessingInstruction cases.
+        (WebCore::Range::checkNodeWOffset): Removed a FIXME - yes, I think that we are supposed
+        to use ProcessingInstruction.data.
+
 2007-11-09  Timothy Hatcher  <timothy@apple.com>
 
         Reviewed by Mark Rowe.

diff --git a/WebCore/dom/Range.cpp b/WebCore/dom/Range.cpp
index 7f51e2c..2e4419e 100644 (file)
--- a/WebCore/dom/Range.cpp
+++ b/WebCore/dom/Range.cpp
@@ -674,7 +674,17 @@ PassRefPtr<DocumentFragment> Range::processContents ( ActionType action, Excepti
             }
         }
         else if (m_startContainer->nodeType() == Node::PROCESSING_INSTRUCTION_NODE) {
-            // ### operate just on data ?
+            if (action == EXTRACT_CONTENTS || action == CLONE_CONTENTS) {
+                RefPtr<ProcessingInstruction> c = static_pointer_cast<ProcessingInstruction>(m_startContainer->cloneNode(true));
+                c->setData(c->data().substring(m_startOffset, m_endOffset - m_startOffset), ec);
+                fragment->appendChild(c.release(), ec);
+            }
+            if (action == EXTRACT_CONTENTS || action == DELETE_CONTENTS) {
+                ProcessingInstruction* pi= static_cast<ProcessingInstruction*>(m_startContainer.get());
+                String data(pi->data());
+                data.remove(m_startOffset, m_endOffset - m_startOffset);
+                pi->setData(data, ec);
+            }
         }
         else {
             Node *n = m_startContainer->firstChild();
@@ -735,8 +745,16 @@ PassRefPtr<DocumentFragment> Range::processContents ( ActionType action, Excepti
             }
         }
         else if (m_startContainer->nodeType() == Node::PROCESSING_INSTRUCTION_NODE) {
-            // ### operate just on data ?
-            // leftContents = ...
+            if (action == EXTRACT_CONTENTS || action == CLONE_CONTENTS) {
+                RefPtr<ProcessingInstruction> c = static_pointer_cast<ProcessingInstruction>(m_startContainer->cloneNode(true));
+                c->setData(c->data().substring(m_startOffset), ec);
+                leftContents = c.release();
+            }
+            if (action == EXTRACT_CONTENTS || action == DELETE_CONTENTS) {
+                ProcessingInstruction* pi= static_cast<ProcessingInstruction*>(m_startContainer.get());
+                String data(pi->data());
+                pi->setData(data.left(m_startOffset), ec);
+            }
         }
         else {
             if (action == EXTRACT_CONTENTS || action == CLONE_CONTENTS)
@@ -797,9 +815,16 @@ PassRefPtr<DocumentFragment> Range::processContents ( ActionType action, Excepti
                 m_startContainer->document()->updateLayout();
             }
         }
-        else if (m_startContainer->nodeType() == Node::PROCESSING_INSTRUCTION_NODE) {
-            // ### operate just on data ?
-            // rightContents = ...
+        else if (m_endContainer->nodeType() == Node::PROCESSING_INSTRUCTION_NODE) {
+            if (action == EXTRACT_CONTENTS || action == CLONE_CONTENTS) {
+                RefPtr<ProcessingInstruction> c = static_pointer_cast<ProcessingInstruction>(m_endContainer->cloneNode(true));
+                c->setData(c->data().left(m_endOffset), ec);
+                rightContents = c.release();
+            }
+            if (action == EXTRACT_CONTENTS || action == DELETE_CONTENTS) {
+                ProcessingInstruction* pi= static_cast<ProcessingInstruction*>(m_endContainer.get());
+                pi->setData(pi->data().substring(m_endOffset), ec);
+            }
         }
         else {
             if (action == EXTRACT_CONTENTS || action == CLONE_CONTENTS)
@@ -1115,11 +1140,11 @@ bool Range::isDetached() const
     return m_detached;
 }
 
-void Range::checkNodeWOffset( Node *n, int offset, ExceptionCode& ec) const
+void Range::checkNodeWOffset(Node* n, int offset, ExceptionCode& ec) const
 {
-    if( offset < 0 ) {
+    if (offset < 0)
         ec = INDEX_SIZE_ERR;
-    }
+        // no return here
 
     switch (n->nodeType()) {
         case Node::ENTITY_NODE:
@@ -1130,16 +1155,15 @@ void Range::checkNodeWOffset( Node *n, int offset, ExceptionCode& ec) const
         case Node::TEXT_NODE:
         case Node::COMMENT_NODE:
         case Node::CDATA_SECTION_NODE:
-            if ( (unsigned)offset > static_cast<CharacterData*>(n)->length() )
+            if ((unsigned)offset > static_cast<CharacterData*>(n)->length())
                 ec = INDEX_SIZE_ERR;
             break;
         case Node::PROCESSING_INSTRUCTION_NODE:
-            // ### are we supposed to check with just data or the whole contents?
-            if ( (unsigned)offset > static_cast<ProcessingInstruction*>(n)->data().length() )
+            if ((unsigned)offset > static_cast<ProcessingInstruction*>(n)->data().length())
                 ec = INDEX_SIZE_ERR;
             break;
         default:
-            if ( (unsigned)offset > n->childNodeCount() )
+            if ((unsigned)offset > n->childNodeCount())
                 ec = INDEX_SIZE_ERR;
             break;
     }