git://git.webkit.org / WebKit-https.git / commitdiff
? search:
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 8e6653c)
Null dereference loading Blink layout test editing/execCommand/delete-hidden-crash...
authorjiewen_tan@apple.com <jiewen_tan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 5 Nov 2015 00:46:34 +0000 (00:46 +0000)
committerjiewen_tan@apple.com <jiewen_tan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 5 Nov 2015 00:46:34 +0000 (00:46 +0000)
https://bugs.webkit.org/show_bug.cgi?id=149289
<rdar://problem/22746352>

Reviewed by Enrica Casucci.

Source/WebCore:

This is a merge of Blink r176497:
https://codereview.chromium.org/340713003

It ensures the start & end positions in DeleteSelectionCommand::initializePositionData
are editable.

Test: editing/execCommand/delete-hidden-crash.html

* editing/DeleteSelectionCommand.cpp:
(WebCore::DeleteSelectionCommand::initializePositionData):
* editing/Editor.cpp:
(WebCore::Editor::advanceToNextMisspelling):
* editing/htmlediting.cpp:
(WebCore::firstEditablePositionAfterPositionInRoot):
(WebCore::lastEditablePositionBeforePositionInRoot):
These two functions don't make any sense to return VisiblePosition. Change them
to return Position instead. Since there is a viable conversion from Position to
VisiblePosition. It should not change the behavior of any other components depending
on it.
* editing/htmlediting.h:

LayoutTests:

* editing/execCommand/delete-hidden-crash-expected.txt: Added.
* editing/execCommand/delete-hidden-crash.html: Added.

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

LayoutTests/ChangeLog patch | blob | history
LayoutTests/editing/execCommand/delete-hidden-crash-expected.txt [new file with mode: 0644] patch | blob
LayoutTests/editing/execCommand/delete-hidden-crash.html [new file with mode: 0644] patch | blob
Source/WebCore/ChangeLog patch | blob | history
Source/WebCore/editing/DeleteSelectionCommand.cpp patch | blob | history
Source/WebCore/editing/Editor.cpp patch | blob | history
Source/WebCore/editing/htmlediting.cpp patch | blob | history
Source/WebCore/editing/htmlediting.h patch | blob | history

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 2e4ddd68d6dd2e4d78e3aa3b9371b4e11e2cad3a..b3dac1cf1d8ff3d12f30371d7ff319b2e767919e 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,14 @@
+2015-11-04  Jiewen Tan  <jiewen_tan@apple.com>
+
+        Null dereference loading Blink layout test editing/execCommand/delete-hidden-crash.html
+        https://bugs.webkit.org/show_bug.cgi?id=149289
+        <rdar://problem/22746352>
+
+        Reviewed by Enrica Casucci.
+
+        * editing/execCommand/delete-hidden-crash-expected.txt: Added.
+        * editing/execCommand/delete-hidden-crash.html: Added.
+
 2015-11-03  Myles C. Maxfield  <mmaxfield@apple.com>
 
         Ruby base ending in tatechuyoko forces a line break before the tatechuyoko
diff --git a/LayoutTests/editing/execCommand/delete-hidden-crash-expected.txt b/LayoutTests/editing/execCommand/delete-hidden-crash-expected.txt
new file mode 100644 (file)
index 0000000..2afa0bf
--- /dev/null
+++ b/LayoutTests/editing/execCommand/delete-hidden-crash-expected.txt
@@ -0,0 +1 @@
+PASS. WebKit didn't crash.
diff --git a/LayoutTests/editing/execCommand/delete-hidden-crash.html b/LayoutTests/editing/execCommand/delete-hidden-crash.html
new file mode 100644 (file)
index 0000000..9034b0f
--- /dev/null
+++ b/LayoutTests/editing/execCommand/delete-hidden-crash.html
@@ -0,0 +1,32 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../editing.js"></script>
+
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+function editingTest() {
+    document.execCommand('SelectAll')
+    span = document.getElementsByTagName('span')[0];
+    span.contentEditable = 'true';
+    span.textContent = 'bar';
+    document.execCommand('InsertText', false, '1');
+    document.body.textContent = "PASS. WebKit didn't crash.";
+};
+</script>
+
+<style>
+* { visibility:visible; }
+.inline { display:inline; }
+* :only-child { visibility:hidden; }
+</style>
+</head>
+<body onload=editingTest()>
+<table>
+<tr><td><span><table></table><span></span></span></td></tr>
+<tr><td><td class="inline">foo</td></tr>
+</table>
+</body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index fc491ae0f68b6402f3dbf147d8bdaff1bb63cc25..00edd4d4b13731a00dc13564d14e71ea11658b87 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,32 @@
+2015-11-04  Jiewen Tan  <jiewen_tan@apple.com>
+
+        Null dereference loading Blink layout test editing/execCommand/delete-hidden-crash.html
+        https://bugs.webkit.org/show_bug.cgi?id=149289
+        <rdar://problem/22746352>
+
+        Reviewed by Enrica Casucci.
+
+        This is a merge of Blink r176497:
+        https://codereview.chromium.org/340713003
+
+        It ensures the start & end positions in DeleteSelectionCommand::initializePositionData
+        are editable.
+
+        Test: editing/execCommand/delete-hidden-crash.html
+
+        * editing/DeleteSelectionCommand.cpp:
+        (WebCore::DeleteSelectionCommand::initializePositionData):
+        * editing/Editor.cpp:
+        (WebCore::Editor::advanceToNextMisspelling):
+        * editing/htmlediting.cpp:
+        (WebCore::firstEditablePositionAfterPositionInRoot):
+        (WebCore::lastEditablePositionBeforePositionInRoot):
+        These two functions don't make any sense to return VisiblePosition. Change them
+        to return Position instead. Since there is a viable conversion from Position to
+        VisiblePosition. It should not change the behavior of any other components depending
+        on it.
+        * editing/htmlediting.h:
+
 2015-11-03  Myles C. Maxfield  <mmaxfield@apple.com>
 
         Ruby base ending in tatechuyoko forces a line break before the tatechuyoko
diff --git a/Source/WebCore/editing/DeleteSelectionCommand.cpp b/Source/WebCore/editing/DeleteSelectionCommand.cpp
index 9689e19ba0b46c0c96b96bd22b0a2ee42d474084..118ce36f99fbef3fef0c8e012b52d95294edc487 100644 (file)
--- a/Source/WebCore/editing/DeleteSelectionCommand.cpp
+++ b/Source/WebCore/editing/DeleteSelectionCommand.cpp
@@ -177,6 +177,11 @@ void DeleteSelectionCommand::initializePositionData()
     Position start, end;
     initializeStartEnd(start, end);
     
+    if (!isEditablePosition(start, ContentIsEditable))
+        start = firstEditablePositionAfterPositionInRoot(start, highestEditableRoot(start));
+    if (!isEditablePosition(end, ContentIsEditable))
+        end = lastEditablePositionBeforePositionInRoot(end, highestEditableRoot(start));
+
     m_upstreamStart = start.upstream();
     m_downstreamStart = start.downstream();
     m_upstreamEnd = end.upstream();
diff --git a/Source/WebCore/editing/Editor.cpp b/Source/WebCore/editing/Editor.cpp
index ed0de379c4fdd0bb3eb5b5f0a846a639c49ad70e..d4b0fadb6e5b75543af08966b43e244cfc51c719 100644 (file)
--- a/Source/WebCore/editing/Editor.cpp
+++ b/Source/WebCore/editing/Editor.cpp
@@ -1902,7 +1902,7 @@ void Editor::advanceToNextMisspelling(bool startBeforeSelection)
         // when spell checking the whole document before sending the message.
         // In that case the document might not be editable, but there are editable pockets that need to be spell checked.
 
-        position = firstEditablePositionAfterPositionInRoot(position, document().documentElement()).deepEquivalent();
+        position = VisiblePosition(firstEditablePositionAfterPositionInRoot(position, document().documentElement())).deepEquivalent();
         if (position.isNull())
             return;
         
diff --git a/Source/WebCore/editing/htmlediting.cpp b/Source/WebCore/editing/htmlediting.cpp
index 9229d79deb7254c95a1d52df1eae46768a779066..f949092518c3e66de763596aa12ed70127ace61f 100644 (file)
--- a/Source/WebCore/editing/htmlediting.cpp
+++ b/Source/WebCore/editing/htmlediting.cpp
@@ -285,7 +285,7 @@ Position previousVisuallyDistinctCandidate(const Position& position)
     return Position();
 }
 
-VisiblePosition firstEditablePositionAfterPositionInRoot(const Position& position, Node* highestRoot)
+Position firstEditablePositionAfterPositionInRoot(const Position& position, Node* highestRoot)
 {
     // position falls before highestRoot.
     if (comparePositions(position, firstPositionInNode(highestRoot)) == -1 && highestRoot->hasEditableStyle())
@@ -296,7 +296,7 @@ VisiblePosition firstEditablePositionAfterPositionInRoot(const Position& positio
     if (&position.deprecatedNode()->treeScope() != &highestRoot->treeScope()) {
         Node* shadowAncestor = highestRoot->treeScope().ancestorInThisScope(p.deprecatedNode());
         if (!shadowAncestor)
-            return VisiblePosition();
+            return Position();
 
         p = positionAfterNode(shadowAncestor);
     }
@@ -305,12 +305,12 @@ VisiblePosition firstEditablePositionAfterPositionInRoot(const Position& positio
         p = isAtomicNode(p.deprecatedNode()) ? positionInParentAfterNode(p.deprecatedNode()) : nextVisuallyDistinctCandidate(p);
     
     if (p.deprecatedNode() && p.deprecatedNode() != highestRoot && !p.deprecatedNode()->isDescendantOf(highestRoot))
-        return VisiblePosition();
+        return Position();
     
-    return VisiblePosition(p);
+    return p;
 }
 
-VisiblePosition lastEditablePositionBeforePositionInRoot(const Position& position, Node* highestRoot)
+Position lastEditablePositionBeforePositionInRoot(const Position& position, Node* highestRoot)
 {
     // When position falls after highestRoot, the result is easy to compute.
     if (comparePositions(position, lastPositionInNode(highestRoot)) == 1)
@@ -321,7 +321,7 @@ VisiblePosition lastEditablePositionBeforePositionInRoot(const Position& positio
     if (&position.deprecatedNode()->treeScope() != &highestRoot->treeScope()) {
         Node* shadowAncestor = highestRoot->treeScope().ancestorInThisScope(p.deprecatedNode());
         if (!shadowAncestor)
-            return VisiblePosition();
+            return Position();
 
         p = firstPositionInOrBeforeNode(shadowAncestor);
     }
@@ -330,9 +330,9 @@ VisiblePosition lastEditablePositionBeforePositionInRoot(const Position& positio
         p = isAtomicNode(p.deprecatedNode()) ? positionInParentBeforeNode(p.deprecatedNode()) : previousVisuallyDistinctCandidate(p);
     
     if (p.deprecatedNode() && p.deprecatedNode() != highestRoot && !p.deprecatedNode()->isDescendantOf(highestRoot))
-        return VisiblePosition();
+        return Position();
     
-    return VisiblePosition(p);
+    return p;
 }
 
 // FIXME: The method name, comment, and code say three different things here!
diff --git a/Source/WebCore/editing/htmlediting.h b/Source/WebCore/editing/htmlediting.h
index 10ced026b0af26ec716ec859ac078a411eb0d334..9fe5f4760b541282fa966da734773661404cecb8 100644 (file)
--- a/Source/WebCore/editing/htmlediting.h
+++ b/Source/WebCore/editing/htmlediting.h
@@ -161,6 +161,9 @@ inline Position lastPositionInOrAfterNode(Node* node)
     return editingIgnoresContent(node) ? positionAfterNode(node) : lastPositionInNode(node);
 }
 
+Position firstEditablePositionAfterPositionInRoot(const Position&, Node*);
+Position lastEditablePositionBeforePositionInRoot(const Position&, Node*);
+
 // comparision functions on Position
     
 int comparePositions(const Position&, const Position&);
@@ -185,9 +188,7 @@ void updatePositionForNodeRemoval(Position&, Node&);
 // -------------------------------------------------------------------------
     
 // Functions returning VisiblePosition
-    
-VisiblePosition firstEditablePositionAfterPositionInRoot(const Position&, Node*);
-VisiblePosition lastEditablePositionBeforePositionInRoot(const Position&, Node*);
+
 VisiblePosition visiblePositionBeforeNode(Node*);
 VisiblePosition visiblePositionAfterNode(Node*);
 
Mirror of the WebKit open source project from svn.webkit.org.