Changeset 31090 in webkit

Timestamp:

Mar 16, 2008, 8:50:33 PM (17 years ago)

Author:

mjs@apple.com

Message:

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/dom/Range/acid3-surround-contents-expected.txt (added)
• LayoutTests/fast/dom/Range/acid3-surround-contents.html (added)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/dom/Range.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/WebCore/dom/Range.cpp

@@ -1406 +1406 @@
     }
 
+    // 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 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;
+        return;
+    }
+    
+    if (m_start.container == newParent || m_start.container->isDescendantOf(newParent.get())) {
+        ec = HIERARCHY_REQUEST_ERR;
+        return;
+    }
+
+    // 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) {
@@ -1419 +1439 @@
         }
     }    
-
-    // 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())
-        parentOfNewParent = parentOfNewParent->parentNode();
-    if (!parentOfNewParent->childTypeAllowed(newParent->nodeType())) {
-        ec = HIERARCHY_REQUEST_ERR;
-        return;
-    }
-    
-    if (m_start.container == newParent || m_start.container->isDescendantOf(newParent.get())) {
-        ec = HIERARCHY_REQUEST_ERR;
-        return;
-    }
-
-    // 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?
 
     ec = 0;