[Shadow][Editing] Assertion in VisibleSelection::adjuseSelectionToAvoidCrossingBounda...
authorshinyak@chromium.org <shinyak@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 Jun 2012 02:02:38 +0000 (02:02 +0000)
committershinyak@chromium.org <shinyak@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 Jun 2012 02:02:38 +0000 (02:02 +0000)
https://bugs.webkit.org/show_bug.cgi?id=89081

Reviewed by Ryosuke Niwa.

Source/WebCore:

firstEditablePositionAfterPositionInRoot and lastEditablePositionBeforePositionInRoot did not
consider a case that an argument hiehestRoot can be in Shadow DOM. So when adjusting selection to
avoid crossing editing boundaries, VisiblePosition can break shadow boundaries, and it causes
an assertion trigger.

By this patch, firstEditablePositionAfterPositionInRoot and lastEditablePositionBeforePositionInRoot will
adjust position to the tree scope of highestRoot instead of its parent tree scope.

Test: editing/shadow/adjusting-editing-boundary-with-table-in-shadow.html

* editing/htmlediting.cpp:
(WebCore::firstEditablePositionAfterPositionInRoot):
(WebCore::lastEditablePositionBeforePositionInRoot):

LayoutTests:

* editing/shadow/adjusting-editing-boundary-with-table-in-shadow-expected.txt: Added.
* editing/shadow/adjusting-editing-boundary-with-table-in-shadow.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/editing/shadow/adjusting-editing-boundary-with-table-in-shadow-expected.txt [new file with mode: 0644]
LayoutTests/editing/shadow/adjusting-editing-boundary-with-table-in-shadow.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/editing/htmlediting.cpp

index b949354..571137b 100644 (file)
@@ -1,3 +1,13 @@
+2012-06-21  Shinya Kawanaka  <shinyak@chromium.org>
+
+        [Shadow][Editing] Assertion in VisibleSelection::adjuseSelectionToAvoidCrossingBoundaries() is triggered.
+        https://bugs.webkit.org/show_bug.cgi?id=89081
+
+        Reviewed by Ryosuke Niwa.
+
+        * editing/shadow/adjusting-editing-boundary-with-table-in-shadow-expected.txt: Added.
+        * editing/shadow/adjusting-editing-boundary-with-table-in-shadow.html: Added.
+
 2012-06-21  Gregg Tavares  <gman@google.com>
 
         Make GL error messages consistent in LayoutTests
diff --git a/LayoutTests/editing/shadow/adjusting-editing-boundary-with-table-in-shadow-expected.txt b/LayoutTests/editing/shadow/adjusting-editing-boundary-with-table-in-shadow-expected.txt
new file mode 100644 (file)
index 0000000..999c84e
--- /dev/null
@@ -0,0 +1,3 @@
+Selecting around a table which is distributed from shadow subtree to nested shadow subtree will trigger an assertion. To try manually, select from "shadow 2" to around "after" and confirm a crash does not occur.
+
+PASS
diff --git a/LayoutTests/editing/shadow/adjusting-editing-boundary-with-table-in-shadow.html b/LayoutTests/editing/shadow/adjusting-editing-boundary-with-table-in-shadow.html
new file mode 100644 (file)
index 0000000..dd7706f
--- /dev/null
@@ -0,0 +1,43 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../fast/dom/shadow/resources/polyfill.js"></script>
+<script src="../../fast/dom/resources/event-sender-util.js"></script>
+
+<div id="container" contenteditable>
+    <table border="1" id="host" contenteditable>
+        <tr><td>host 1</td></tr>
+        <tr><td>host 2</td></tr>
+    </table>
+</div>
+
+<p id="description">Selecting around a table which is distributed from shadow subtree to nested shadow subtree will trigger an assertion. To try manually, select from "shadow 2" to around "after" and confirm a crash does not occur.</p>
+
+<script>
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+
+var shadowRoot = new WebKitShadowRoot(host);
+var div = document.createElement('div');
+shadowRoot.appendChild(div);
+div.innerHTML = "<table border='1'><tr><td>shadow 1</td></tr><tr><td id='src'>shadow 2</td></tr></table>";
+
+var nestedShadowRoot = new WebKitShadowRoot(div);
+nestedShadowRoot.innerHTML = "<div contenteditable>before<shadow></shadow>after</div>";
+
+var src = shadowRoot.getElementById('src');
+
+if (window.eventSender) {
+    eventSender.mouseMoveTo(src.offsetLeft + 10, src.offsetTop + src.offsetHeight / 2);
+    eventSender.mouseDown();
+    eventSender.mouseMoveTo(src.offsetLeft - 10, src.offsetTop + src.offsetHeight / 2);
+    eventSender.mouseUp();
+
+    container.innerHTML = "PASS";
+}
+
+// When description comes before container, this test will not fail if code is wrong. So let's move here.
+container.parentNode.insertBefore(description, container);
+</script>
+</body>
+</html>
index 1ab748a..36013e5 100644 (file)
@@ -1,3 +1,24 @@
+2012-06-21  Shinya Kawanaka  <shinyak@chromium.org>
+
+        [Shadow][Editing] Assertion in VisibleSelection::adjuseSelectionToAvoidCrossingBoundaries() is triggered.
+        https://bugs.webkit.org/show_bug.cgi?id=89081
+
+        Reviewed by Ryosuke Niwa.
+
+        firstEditablePositionAfterPositionInRoot and lastEditablePositionBeforePositionInRoot did not
+        consider a case that an argument hiehestRoot can be in Shadow DOM. So when adjusting selection to
+        avoid crossing editing boundaries, VisiblePosition can break shadow boundaries, and it causes
+        an assertion trigger.
+
+        By this patch, firstEditablePositionAfterPositionInRoot and lastEditablePositionBeforePositionInRoot will
+        adjust position to the tree scope of highestRoot instead of its parent tree scope.
+
+        Test: editing/shadow/adjusting-editing-boundary-with-table-in-shadow.html
+
+        * editing/htmlediting.cpp:
+        (WebCore::firstEditablePositionAfterPositionInRoot):
+        (WebCore::lastEditablePositionBeforePositionInRoot):
+
 2012-06-21  Andrey Kosyakov  <caseq@chromium.org>
 
         Web Inspector: exception in TimelinePresentationModel when recording timeline
index e959178..7d8fc39 100644 (file)
@@ -259,11 +259,15 @@ VisiblePosition firstEditablePositionAfterPositionInRoot(const Position& positio
         return firstPositionInNode(highestRoot);
 
     Position p = position;
-    
-    if (Node* shadowAncestor = p.deprecatedNode()->shadowAncestorNode())
-        if (shadowAncestor != p.deprecatedNode())
-            p = positionAfterNode(shadowAncestor);
-    
+
+    if (position.deprecatedNode()->treeScope() != highestRoot->treeScope()) {
+        Node* shadowAncestor = highestRoot->treeScope()->ancestorInThisScope(p.deprecatedNode());
+        if (!shadowAncestor)
+            return VisiblePosition();
+
+        p = positionAfterNode(shadowAncestor);
+    }
+
     while (p.deprecatedNode() && !isEditablePosition(p) && p.deprecatedNode()->isDescendantOf(highestRoot))
         p = isAtomicNode(p.deprecatedNode()) ? positionInParentAfterNode(p.deprecatedNode()) : nextVisuallyDistinctCandidate(p);
     
@@ -281,9 +285,12 @@ VisiblePosition lastEditablePositionBeforePositionInRoot(const Position& positio
 
     Position p = position;
 
-    if (Node* shadowAncestor = p.deprecatedNode()->shadowAncestorNode()) {
-        if (shadowAncestor != p.deprecatedNode())
-            p = firstPositionInOrBeforeNode(shadowAncestor);
+    if (position.deprecatedNode()->treeScope() != highestRoot->treeScope()) {
+        Node* shadowAncestor = highestRoot->treeScope()->ancestorInThisScope(p.deprecatedNode());
+        if (!shadowAncestor)
+            return VisiblePosition();
+
+        p = firstPositionInOrBeforeNode(shadowAncestor);
     }
     
     while (p.deprecatedNode() && !isEditablePosition(p) && p.deprecatedNode()->isDescendantOf(highestRoot))