WebCore:

2008-03-16  Maciej Stachowiak  <mjs@apple.com>

        Reviewed by Darin.

        - fixed "Acid3 expects different exceptions for surroundContents calls involving comment nodes (affects Acid3 test 11)"
        http://bugs.webkit.org/show_bug.cgi?id=17509

        This gets us to 92/100

        * dom/Range.cpp:
        (WebCore::Range::surroundContents): Check for
        HIERARCHY_REQUEST_ERR before BAD_BOUNDARYPOINTS_ERR, since Acid3
        expects exceptional conditions to be tested in the order that the
        spec lists them. Also, adjust the HIERARCHY_REQUEST_ERR check. If
        the start point of the range is in a comment node, the node that
        would be the parent of a partial replacement is actually the
        comment node's parent (since comment nodes have character
        indices), so we should do the HIERARCHY_REQUEST_ERR check based on
        the parent of the comment node, as for text nodes, even though it
        will fail later with a different exception because it is not
        allowed to surround a partially selected non-text node.

LayoutTests:

2008-03-16  Maciej Stachowiak  <mjs@apple.com>

        Reviewed by Darin.

        - test for "Acid3 expects different exceptions for surroundContents calls involving comment nodes (affects Acid3 test 11)"
        http://bugs.webkit.org/show_bug.cgi?id=17509

        * fast/dom/Range/acid3-surround-contents-expected.txt: Added.
        * fast/dom/Range/acid3-surround-contents.html: Added.



Canonical link: https://commits.webkit.org/24773@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@31090 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog

@@ -1,3 +1,13 @@
+2008-03-16  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Darin.
+
+        - test for "Acid3 expects different exceptions for surroundContents calls involving comment nodes (affects Acid3 test 11)"
+        http://bugs.webkit.org/show_bug.cgi?id=17509
+
+        * fast/dom/Range/acid3-surround-contents-expected.txt: Added.
+        * fast/dom/Range/acid3-surround-contents.html: Added.
+
 2008-03-16  Marvin Decker  <marv.decker@gmail.com>
 
         Reviewed by Darin.

LayoutTests/fast/dom/Range/acid3-surround-contents-expected.txt

@@ -0,0 +1,5 @@
+
+The test below should report no failures, and should say PASS at the end.
+
+PASS
+

LayoutTests/fast/dom/Range/acid3-surround-contents.html

@@ -0,0 +1,81 @@
+<iframe src="empty.html" id="selectors" width=0 height=0 frameborder=0></iframe>
+<p>The test below should report no failures, and should say PASS at the end.</p>
+<script>
+if (window.layoutTestController) {
+    layoutTestController.dumpAsText();
+}
+</script>
+<script>
+
+  function getTestDocument() {
+    var iframe = document.getElementById("selectors");
+    var doc = iframe.contentDocument;
+    for (var i = doc.documentElement.childNodes.length-1; i >= 0; i -= 1)
+      doc.documentElement.removeChild(doc.documentElement.childNodes[i]);
+    doc.documentElement.appendChild(doc.createElement('head'));
+    doc.documentElement.firstChild.appendChild(doc.createElement('title'));
+    doc.documentElement.appendChild(doc.createElement('body'));
+    return doc;
+  }
+
+var failCount = 0;
+
+  function fail(message) {
+    document.write(message.replace("&", "&amp;").replace("<", "&lt;") + "<br>");
+    ++failCount;
+  }
+
+  function assert(condition, message) {
+    if (!condition)
+      fail(message);
+  }
+
+  function assertEquals(expression, value, message) {
+    if (expression != value) {
+      expression = (""+expression).replace(/[\r\n]+/g, "\\n");
+      value = (""+value).replace(/\r?\n/g, "\\n");
+      fail("expected '" + value + "' but got '" + expression + "' - " + message);
+    }
+  }
+
+      // test 11: Ranges and Comments
+      var msg;
+      var doc = getTestDocument();
+      var c1 = doc.createComment("11111");
+      doc.appendChild(c1);
+      var r = doc.createRange();
+      r.selectNode(c1);
+      msg = 'wrong exception raised';
+      try {
+        r.surroundContents(doc.createElement('a'));
+        msg = 'no exception raised';
+      } catch (e) {
+        if ('code' in e)
+          msg += '; code = ' + e.code;
+        if (e.code == 3)
+          msg = '';
+      }
+      assert(msg == '', "when inserting <a> into Document with another child: " + msg);
+      var c2 = doc.createComment("22222");
+      doc.body.appendChild(c2);
+      var c3 = doc.createComment("33333");
+      doc.body.appendChild(c3);
+      r.setStart(c2, 2);
+      r.setEnd(c3, 3);
+      var msg = 'wrong exception raised';
+      try {
+        r.surroundContents(doc.createElement('a'));
+        msg = 'no exception raised';
+      } catch (e) {
+        if ('code' in e)
+          msg += '; code = ' + e.code;
+        if (e.code == 1)
+          msg = '';
+      }
+      assert(msg == '', "when trying to surround two halves of comment: " + msg);
+      assertEquals(r.toString(), "", "comments returned text");
+
+if (failCount == 0)
+  document.write("PASS<br>");
+</script>
+

WebCore/ChangeLog

@@ -1,3 +1,25 @@
+2008-03-16  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Darin.
+
+        - fixed "Acid3 expects different exceptions for surroundContents calls involving comment nodes (affects Acid3 test 11)"
+        http://bugs.webkit.org/show_bug.cgi?id=17509
+
+        This gets us to 92/100
+
+        * dom/Range.cpp:
+        (WebCore::Range::surroundContents): Check for
+        HIERARCHY_REQUEST_ERR before BAD_BOUNDARYPOINTS_ERR, since Acid3
+        expects exceptional conditions to be tested in the order that the
+        spec lists them. Also, adjust the HIERARCHY_REQUEST_ERR check. If
+        the start point of the range is in a comment node, the node that
+        would be the parent of a partial replacement is actually the
+        comment node's parent (since comment nodes have character
+        indices), so we should do the HIERARCHY_REQUEST_ERR check based on
+        the parent of the comment node, as for text nodes, even though it
+        will fail later with a different exception because it is not
+        allowed to surround a partially selected non-text node.
+
 2008-03-16  Marvin Decker  <marv.decker@gmail.com>
 
         Reviewed by Darin.

WebCore/dom/Range.cpp

@@ -1405,25 +1405,12 @@ void Range::surroundContents(PassRefPtr<Node> passNewParent, ExceptionCode& ec)
         return;
     }
 
-    // BAD_BOUNDARYPOINTS_ERR: Raised if the Range partially selects a non-Text node.
-    if (m_start.container->nodeType() != Node::TEXT_NODE) {
-        if (m_start.offset > 0 && m_start.offset < maxStartOffset()) {
-            ec = RangeException::BAD_BOUNDARYPOINTS_ERR;
-            return;
-        }
-    }
-    if (m_end.container->nodeType() != Node::TEXT_NODE) {
-        if (m_end.offset > 0 && m_end.offset < maxEndOffset()) {
-            ec = RangeException::BAD_BOUNDARYPOINTS_ERR;
-            return;
-        }
-    }    
-
     // Raise a HIERARCHY_REQUEST_ERR if m_start.container doesn't accept children like newParent.
     Node* parentOfNewParent = m_start.container.get();
-    // If m_start.container is a textNode, it will be split and it will be its parent that will 
-    // need to accept newParent.
-    if (parentOfNewParent->isTextNode())
+
+    // If m_start.container is a character data node, it will be split and it will be its parent that will 
+    // need to accept newParent (or in the case of a comment, it logically "would"
+    if (parentOfNewParent->isCharacterDataNode())
         parentOfNewParent = parentOfNewParent->parentNode();
     if (!parentOfNewParent->childTypeAllowed(newParent->nodeType())) {
         ec = HIERARCHY_REQUEST_ERR;
@@ -1438,6 +1425,20 @@ void Range::surroundContents(PassRefPtr<Node> passNewParent, ExceptionCode& ec)
     // FIXME: Do we need a check if the node would end up with a child node of a type not
     // allowed by the type of node?
 
+    // BAD_BOUNDARYPOINTS_ERR: Raised if the Range partially selects a non-Text node.
+    if (m_start.container->nodeType() != Node::TEXT_NODE) {
+        if (m_start.offset > 0 && m_start.offset < maxStartOffset()) {
+            ec = RangeException::BAD_BOUNDARYPOINTS_ERR;
+            return;
+        }
+    }
+    if (m_end.container->nodeType() != Node::TEXT_NODE) {
+        if (m_end.offset > 0 && m_end.offset < maxEndOffset()) {
+            ec = RangeException::BAD_BOUNDARYPOINTS_ERR;
+            return;
+        }
+    }    
+
     ec = 0;
     while (Node* n = newParent->firstChild()) {
         newParent->removeChild(n, ec);