2010-04-01 MORITA Hajime <morrita@google.com>
authortkent@chromium.org <tkent@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 2 Apr 2010 03:14:12 +0000 (03:14 +0000)
committertkent@chromium.org <tkent@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 2 Apr 2010 03:14:12 +0000 (03:14 +0000)
        Reviewed by Darin Adler.

        WebCore::Document::updateLayoutIgnorePendingStylesheets NULL pointer
        https://bugs.webkit.org/show_bug.cgi?id=31680
        Ownerless nodes leads a crash on DOMSelection APIs
        https://bugs.webkit.org/show_bug.cgi?id=36800

        * editing/selection/DOMSelection-DocumentType-expected.txt: Added.
        * editing/selection/DOMSelection-DocumentType.html: Added.
        * editing/selection/DOMSelection-crossing-document-expected.txt: Added.
        * editing/selection/DOMSelection-crossing-document.html: Added.
        * editing/selection/drag-in-iframe.html:
          Updated to follow follow behaviour change.
        * editing/selection/script-tests/DOMSelection-DocumentType.js: Added.
        * editing/selection/script-tests/DOMSelection-crossing-document.js: Added.
        (makeEditableDocument):
        (clear):
2010-04-01  MORITA Hajime  <morrita@google.com>

        Reviewed by Darin Adler.

        WebCore::Document::updateLayoutIgnorePendingStylesheets NULL pointer
        https://bugs.webkit.org/show_bug.cgi?id=31680
        Ownerless nodes leads a crash on DOMSelection APIs
        https://bugs.webkit.org/show_bug.cgi?id=36800

        Added guards nodes from foreign documents to DOMSelection APIs.

        Tests: editing/selection/DOMSelection-DocumentType.html
               editing/selection/DOMSelection-crossing-document.html

        * editing/VisiblePosition.cpp:
        (WebCore::VisiblePosition::canonicalPosition):
        * page/DOMSelection.cpp:
        (WebCore::DOMSelection::collapse):
        (WebCore::DOMSelection::setBaseAndExtent):
        (WebCore::DOMSelection::setPosition):
        (WebCore::DOMSelection::extend):
        (WebCore::DOMSelection::containsNode):
        (WebCore::DOMSelection::isValidForPosition):
        * page/DOMSelection.h:

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

12 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/selection/DOMSelection-DocumentType-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/DOMSelection-DocumentType.html [new file with mode: 0644]
LayoutTests/editing/selection/DOMSelection-crossing-document-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/DOMSelection-crossing-document.html [new file with mode: 0644]
LayoutTests/editing/selection/drag-in-iframe.html
LayoutTests/editing/selection/script-tests/DOMSelection-DocumentType.js [new file with mode: 0644]
LayoutTests/editing/selection/script-tests/DOMSelection-crossing-document.js [new file with mode: 0644]
WebCore/ChangeLog
WebCore/editing/VisiblePosition.cpp
WebCore/page/DOMSelection.cpp
WebCore/page/DOMSelection.h

index 42fff79..412bb1f 100644 (file)
@@ -1,3 +1,23 @@
+2010-04-01  MORITA Hajime  <morrita@google.com>
+
+        Reviewed by Darin Adler.
+
+        WebCore::Document::updateLayoutIgnorePendingStylesheets NULL pointer
+        https://bugs.webkit.org/show_bug.cgi?id=31680
+        Ownerless nodes leads a crash on DOMSelection APIs
+        https://bugs.webkit.org/show_bug.cgi?id=36800
+
+        * editing/selection/DOMSelection-DocumentType-expected.txt: Added.
+        * editing/selection/DOMSelection-DocumentType.html: Added.
+        * editing/selection/DOMSelection-crossing-document-expected.txt: Added.
+        * editing/selection/DOMSelection-crossing-document.html: Added.
+        * editing/selection/drag-in-iframe.html:
+          Updated to follow follow behaviour change.
+        * editing/selection/script-tests/DOMSelection-DocumentType.js: Added.
+        * editing/selection/script-tests/DOMSelection-crossing-document.js: Added.
+        (makeEditableDocument):
+        (clear):
+
 2010-04-01  Chris Evans  <cevans@chromium.org>
 
         Reviewed by Adam Barth.
diff --git a/LayoutTests/editing/selection/DOMSelection-DocumentType-expected.txt b/LayoutTests/editing/selection/DOMSelection-DocumentType-expected.txt
new file mode 100644 (file)
index 0000000..2e98623
--- /dev/null
@@ -0,0 +1,16 @@
+Test to check if setBaseAndExtent guard node with null owner document (Bug 31680)
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS sel.anchorNode is null
+PASS sel.anchorNode is null
+PASS sel.anchorNode is null
+PASS sel.anchorNode is null
+PASS sel.anchorNode is null
+PASS sel.anchorNode is null
+PASS sel.containsNode(docType) is false
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/editing/selection/DOMSelection-DocumentType.html b/LayoutTests/editing/selection/DOMSelection-DocumentType.html
new file mode 100644 (file)
index 0000000..754df18
--- /dev/null
@@ -0,0 +1,14 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href="../../fast/js/resources/js-test-style.css">
+<script src="../../fast/js/resources/js-test-pre.js"></script>
+<script src="resources/js-test-selection-shared.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script src="script-tests/DOMSelection-DocumentType.js"></script>
+<script src="../../fast/js/resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/editing/selection/DOMSelection-crossing-document-expected.txt b/LayoutTests/editing/selection/DOMSelection-crossing-document-expected.txt
new file mode 100644 (file)
index 0000000..b994516
--- /dev/null
@@ -0,0 +1,22 @@
+Test to check if setBaseAndExtent guard node with null owner document (Bug 31680)
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS foreignSel.anchorNode is null
+PASS mainSel.anchorNode is null
+PASS foreignSel.anchorNode is null
+PASS mainSel.anchorNode is null
+PASS foreignSel.anchorNode is null
+PASS mainSel.anchorNode is null
+PASS foreignSel.anchorNode is null
+PASS mainSel.anchorNode is null
+PASS foreignSel.anchorNode is null
+PASS mainSel.anchorNode is null
+PASS foreignSel.anchorNode is null
+PASS mainSel.anchorNode is null
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
+Main Text
diff --git a/LayoutTests/editing/selection/DOMSelection-crossing-document.html b/LayoutTests/editing/selection/DOMSelection-crossing-document.html
new file mode 100644 (file)
index 0000000..d51f272
--- /dev/null
@@ -0,0 +1,14 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href="../../fast/js/resources/js-test-style.css">
+<script src="../../fast/js/resources/js-test-pre.js"></script>
+<script src="resources/js-test-selection-shared.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script src="script-tests/DOMSelection-crossing-document.js"></script>
+<script src="../../fast/js/resources/js-test-post.js"></script>
+</body>
+</html>
index 05940c0..e12d4af 100644 (file)
@@ -15,8 +15,8 @@
             var x1 = iframe.offsetLeft + target.offsetLeft + target.offsetWidth / 2;
             var x2 = iframe.offsetLeft + iframe.offsetWidth - 20;
             var y = iframe.offsetTop + target.offsetTop + target.offsetHeight / 2;
-        
-            window.getSelection().setBaseAndExtent(target, 0, target, 1);
+
+            iframe.contentWindow.getSelection().setBaseAndExtent(target, 0, target, 1);
         
             eventSender.mouseMoveTo(x1, y);
             eventSender.dragMode = false;
diff --git a/LayoutTests/editing/selection/script-tests/DOMSelection-DocumentType.js b/LayoutTests/editing/selection/script-tests/DOMSelection-DocumentType.js
new file mode 100644 (file)
index 0000000..79545dd
--- /dev/null
@@ -0,0 +1,26 @@
+description("Test to check if setBaseAndExtent guard node with null owner document (Bug 31680)");
+
+var sel = window.getSelection();
+var docType = document.implementation.createDocumentType('c');
+
+sel.setBaseAndExtent(docType);
+shouldBeNull("sel.anchorNode");
+
+sel.setBaseAndExtent(null, 0, docType, 0);
+shouldBeNull("sel.anchorNode");
+
+sel.collapse(docType);
+shouldBeNull("sel.anchorNode");
+
+sel.selectAllChildren(docType);
+shouldBeNull("sel.anchorNode");
+
+sel.extend(docType, 0);
+shouldBeNull("sel.anchorNode");
+
+sel.containsNode(docType);
+shouldBeNull("sel.anchorNode");
+
+shouldBeFalse("sel.containsNode(docType)");
+
+var successfullyParsed = true;
diff --git a/LayoutTests/editing/selection/script-tests/DOMSelection-crossing-document.js b/LayoutTests/editing/selection/script-tests/DOMSelection-crossing-document.js
new file mode 100644 (file)
index 0000000..48eea43
--- /dev/null
@@ -0,0 +1,59 @@
+description("Test to check if setBaseAndExtent guard node with null owner document (Bug 31680)");
+
+function makeEditableDocument(id)
+{
+    var iframe = document.createElement("iframe");
+    document.body.appendChild(iframe);
+    var doc = iframe.contentDocument;
+    doc.body.innerHTML = "<html><body><div id='" + id + "' contentEditable>Editable Block for " + id + ".</div></body></html>";
+    return doc;
+}
+
+var foreignDoc = makeEditableDocument("target");
+var foreignElement = foreignDoc.getElementById("target");
+var foreignText = foreignElement.firstChild;
+var foreignSel = foreignDoc.getSelection();
+
+var mainElement = document.createElement("div");
+mainElement.contentEditable = true;
+mainElement.innerHTML = "Main Text";
+document.body.appendChild(mainElement);
+var mainSel = window.getSelection();
+
+function clear()
+{
+    foreignSel.setBaseAndExtent(null, 0, null, 0);
+    mainSel.setBaseAndExtent(null, 0, null, 0);
+}
+
+mainSel.setBaseAndExtent(foreignElement, 0, foreignElement, 0);
+shouldBeNull("foreignSel.anchorNode");
+shouldBeNull("mainSel.anchorNode");
+
+clear();
+mainSel.setPosition(foreignElement, 0);
+shouldBeNull("foreignSel.anchorNode");
+shouldBeNull("mainSel.anchorNode");
+
+clear();
+mainSel.extend(foreignElement, 1);
+shouldBeNull("foreignSel.anchorNode");
+shouldBeNull("mainSel.anchorNode");
+
+clear();
+mainSel.selectAllChildren(foreignElement, 1);
+shouldBeNull("foreignSel.anchorNode");
+shouldBeNull("mainSel.anchorNode");
+
+clear();
+mainSel.collapse(foreignElement, 0);
+shouldBeNull("foreignSel.anchorNode");
+shouldBeNull("mainSel.anchorNode");
+
+// Should not allow elements which come from another document.
+clear();
+mainSel.setBaseAndExtent(mainElement, 0, foreignElement, 0);
+shouldBeNull("foreignSel.anchorNode");
+shouldBeNull("mainSel.anchorNode");
+
+var successfullyParsed = true;
index efd9e13..ea5685d 100644 (file)
@@ -1,3 +1,28 @@
+2010-04-01  MORITA Hajime  <morrita@google.com>
+
+        Reviewed by Darin Adler.
+
+        WebCore::Document::updateLayoutIgnorePendingStylesheets NULL pointer
+        https://bugs.webkit.org/show_bug.cgi?id=31680
+        Ownerless nodes leads a crash on DOMSelection APIs
+        https://bugs.webkit.org/show_bug.cgi?id=36800
+
+        Added guards nodes from foreign documents to DOMSelection APIs.
+
+        Tests: editing/selection/DOMSelection-DocumentType.html
+               editing/selection/DOMSelection-crossing-document.html
+
+        * editing/VisiblePosition.cpp:
+        (WebCore::VisiblePosition::canonicalPosition):
+        * page/DOMSelection.cpp:
+        (WebCore::DOMSelection::collapse):
+        (WebCore::DOMSelection::setBaseAndExtent):
+        (WebCore::DOMSelection::setPosition):
+        (WebCore::DOMSelection::extend):
+        (WebCore::DOMSelection::containsNode):
+        (WebCore::DOMSelection::isValidForPosition):
+        * page/DOMSelection.h:
+
 2010-04-01  Chris Evans  <cevans@chromium.org>
 
         Reviewed by Adam Barth.
index 252998f..1b4a514 100644 (file)
@@ -450,6 +450,7 @@ Position VisiblePosition::canonicalPosition(const Position& position)
     if (!node)
         return Position();
 
+    ASSERT(node->document());
     node->document()->updateLayoutIgnorePendingStylesheets();
 
     Position candidate = position.upstream();
index 12ae59d..a583176 100644 (file)
@@ -207,6 +207,10 @@ void DOMSelection::collapse(Node* node, int offset, ExceptionCode& ec)
         ec = INDEX_SIZE_ERR;
         return;
     }
+
+    if (!isValidForPosition(node))
+        return;
+
     m_frame->selection()->moveTo(VisiblePosition(node, offset, DOWNSTREAM));
 }
 
@@ -244,6 +248,10 @@ void DOMSelection::setBaseAndExtent(Node* baseNode, int baseOffset, Node* extent
         ec = INDEX_SIZE_ERR;
         return;
     }
+
+    if (!isValidForPosition(baseNode) || !isValidForPosition(extentNode))
+        return;
+
     VisiblePosition visibleBase = VisiblePosition(baseNode, baseOffset, DOWNSTREAM);
     VisiblePosition visibleExtent = VisiblePosition(extentNode, extentOffset, DOWNSTREAM);
 
@@ -258,6 +266,10 @@ void DOMSelection::setPosition(Node* node, int offset, ExceptionCode& ec)
         ec = INDEX_SIZE_ERR;
         return;
     }
+
+    if (!isValidForPosition(node))
+        return;
+
     m_frame->selection()->moveTo(VisiblePosition(node, offset, DOWNSTREAM));
 }
 
@@ -320,13 +332,16 @@ void DOMSelection::extend(Node* node, int offset, ExceptionCode& ec)
         ec = TYPE_MISMATCH_ERR;
         return;
     }
+
     if (offset < 0 || offset > (node->offsetInCharacters() ? caretMaxOffset(node) : (int)node->childNodeCount())) {
         ec = INDEX_SIZE_ERR;
         return;
     }
 
-    SelectionController* selection = m_frame->selection();
-    selection->setExtent(VisiblePosition(node, offset, DOWNSTREAM));
+    if (!isValidForPosition(node))
+        return;
+
+    m_frame->selection()->setExtent(VisiblePosition(node, offset, DOWNSTREAM));
 }
 
 PassRefPtr<Range> DOMSelection::getRangeAt(int index, ExceptionCode& ec)
@@ -428,7 +443,7 @@ bool DOMSelection::containsNode(const Node* n, bool allowPartial) const
 
     SelectionController* selection = m_frame->selection();
 
-    if (!n || selection->isNone())
+    if (!n || m_frame->document() != n->document() || selection->isNone())
         return false;
 
     Node* parentNode = n->parentNode();
@@ -471,4 +486,12 @@ String DOMSelection::toString()
     return plainText(m_frame->selection()->selection().toNormalizedRange().get());
 }
 
+bool DOMSelection::isValidForPosition(Node* node) const
+{
+    ASSERT(m_frame);
+    if (!node)
+        return true;
+    return node->document() == m_frame->document();
+}
+
 } // namespace WebCore
index e0fe1e3..0287e44 100644 (file)
@@ -96,6 +96,7 @@ namespace WebCore {
         // Convenience method for accessors, does not NULL check m_frame.
         const VisibleSelection& visibleSelection() const;
 
+        bool isValidForPosition(Node*) const;
         Frame* m_frame;
     };