Title: [120666] trunk
Revision
120666
Author
[email protected]
Date
2012-06-18 21:02:48 -0700 (Mon, 18 Jun 2012)

Log Message

[Crash][Editing] VisibleSelection::adjustSelectionToAvoidCrossingEditingBoundaries() crashes in some Shadow DOM case.
https://bugs.webkit.org/show_bug.cgi?id=88500

Reviewed by Ryosuke Niwa.

Source/WebCore:

When selecting from a child of shadow host to an element in Shadow DOM, the start position of
Selection comes after the end position of Selection in some cases. This caused a crash.

For example, let's think '<div id="host">A</div>D' and host has a Shadow DOM whose innerHTML
is 'B<content></content>C'. When selecting from just after A (p1 = (PositionIsOffsetInAnchor,
#text, 1)) to just before B (p2 = (PositionIsOffsetInAnchor, shadowRoot, 0)), since
comparePosition always judge as p2 is before p1, m_start and m_end in VisibleSelection
will be p2 and p1 respectively.

However, the anchorNodes of p1 and p2 have the different tree scope, we have to adjust p2 to
the same tree scope as p1. Since p2 is m_start, m_start was adjusted to positionAfterNode(div).
This breaks the assumption that m_start is always before (or equal to) m_end.

This patch keeps the assumption. When we cannot move m_start to positionAfterNode(div),
we move m_start to positionBeforeNode(div). Note that we cannot always move m_start to
positionBeforeNode(div), because when selecting just after D (p3 = (PositionIsOffsetInAnchor,
D's parent, 2), we don't want to select A.

In this case, m_start should be adjusted to positionAfterNode(div).

Test: editing/shadow/breaking-editing-boundaries.html

* editing/VisibleSelection.cpp:
(WebCore::adjustPositionForEnd):
(WebCore):
(WebCore::adjustPositionForStart):
(WebCore::VisibleSelection::adjustSelectionToAvoidCrossingShadowBoundaries):
* editing/VisibleSelection.h:
(VisibleSelection):

LayoutTests:

* editing/shadow/breaking-editing-boundaries-expected.txt: Added.
* editing/shadow/breaking-editing-boundaries.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (120665 => 120666)


--- trunk/LayoutTests/ChangeLog	2012-06-19 03:54:51 UTC (rev 120665)
+++ trunk/LayoutTests/ChangeLog	2012-06-19 04:02:48 UTC (rev 120666)
@@ -1,3 +1,13 @@
+2012-06-18  Shinya Kawanaka  <[email protected]>
+
+        [Crash][Editing] VisibleSelection::adjustSelectionToAvoidCrossingEditingBoundaries() crashes in some Shadow DOM case.
+        https://bugs.webkit.org/show_bug.cgi?id=88500
+
+        Reviewed by Ryosuke Niwa.
+
+        * editing/shadow/breaking-editing-boundaries-expected.txt: Added.
+        * editing/shadow/breaking-editing-boundaries.html: Added.
+
 2012-06-18  Hajime Morrita  <[email protected]>
 
         Unreviewed test marking. Marked border-radius-clip-002.htm as a failure.

Added: trunk/LayoutTests/editing/shadow/breaking-editing-boundaries-expected.txt (0 => 120666)


--- trunk/LayoutTests/editing/shadow/breaking-editing-boundaries-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/shadow/breaking-editing-boundaries-expected.txt	2012-06-19 04:02:48 UTC (rev 120666)
@@ -0,0 +1,8 @@
+When selecting from a child of shadow host to an element in Shadow DOM, a crash should not be caused. This is because the start position of Selection comes after the end position of Selection.
+
+before host drag from here after host
+PASS
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/editing/shadow/breaking-editing-boundaries.html (0 => 120666)


--- trunk/LayoutTests/editing/shadow/breaking-editing-boundaries.html	                        (rev 0)
+++ trunk/LayoutTests/editing/shadow/breaking-editing-boundaries.html	2012-06-19 04:02:48 UTC (rev 120666)
@@ -0,0 +1,36 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script src=""
+<script src=""
+
+<p>When selecting from a child of shadow host to an element in Shadow DOM, a crash should not be caused.
+This is because the start position of Selection comes after the end position of Selection.</p>
+
+<div contenteditable>
+    before host <span id="host" contenteditable="false">drag from here<span id="target"> </span> after host</span>
+</div>
+<pre id="console"></pre>
+
+<script>
+var shadowRoot = new WebKitShadowRoot(host);
+shadowRoot.innerHTML = "<span id='shadow-host'>before shadow<shadow id='shadow'></shadow>after shadow</span>"
+
+var nestedShadowHost = shadowRoot.getElementById('shadow-host');
+var nestedShadowRoot = new WebKitShadowRoot(nestedShadowHost);
+nestedShadowRoot.innerHTML = "<span contenteditable>before nested<shadow></shadow>after nested</span>"
+
+// This should not cause a crash.
+eventSender.mouseMoveTo(target.offsetLeft - 10, target.offsetTop + target.offsetHeight / 2);
+eventSender.mouseDown();
+eventSender.mouseMoveTo(nestedShadowHost.offsetLeft + 10, nestedShadowHost.offsetTop + nestedShadowHost.offsetHeight / 2);
+eventSender.mouseUp();
+debug('PASS');
+
+var successfullyParsed = true;
+</script>
+
+<script src=""
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (120665 => 120666)


--- trunk/Source/WebCore/ChangeLog	2012-06-19 03:54:51 UTC (rev 120665)
+++ trunk/Source/WebCore/ChangeLog	2012-06-19 04:02:48 UTC (rev 120666)
@@ -1,3 +1,40 @@
+2012-06-18  Shinya Kawanaka  <[email protected]>
+
+        [Crash][Editing] VisibleSelection::adjustSelectionToAvoidCrossingEditingBoundaries() crashes in some Shadow DOM case.
+        https://bugs.webkit.org/show_bug.cgi?id=88500
+
+        Reviewed by Ryosuke Niwa.
+
+        When selecting from a child of shadow host to an element in Shadow DOM, the start position of
+        Selection comes after the end position of Selection in some cases. This caused a crash.
+
+        For example, let's think '<div id="host">A</div>D' and host has a Shadow DOM whose innerHTML
+        is 'B<content></content>C'. When selecting from just after A (p1 = (PositionIsOffsetInAnchor,
+        #text, 1)) to just before B (p2 = (PositionIsOffsetInAnchor, shadowRoot, 0)), since
+        comparePosition always judge as p2 is before p1, m_start and m_end in VisibleSelection
+        will be p2 and p1 respectively.
+
+        However, the anchorNodes of p1 and p2 have the different tree scope, we have to adjust p2 to
+        the same tree scope as p1. Since p2 is m_start, m_start was adjusted to positionAfterNode(div).
+        This breaks the assumption that m_start is always before (or equal to) m_end.
+
+        This patch keeps the assumption. When we cannot move m_start to positionAfterNode(div),
+        we move m_start to positionBeforeNode(div). Note that we cannot always move m_start to
+        positionBeforeNode(div), because when selecting just after D (p3 = (PositionIsOffsetInAnchor,
+        D's parent, 2), we don't want to select A.
+
+        In this case, m_start should be adjusted to positionAfterNode(div).
+
+        Test: editing/shadow/breaking-editing-boundaries.html
+
+        * editing/VisibleSelection.cpp:
+        (WebCore::adjustPositionForEnd):
+        (WebCore):
+        (WebCore::adjustPositionForStart):
+        (WebCore::VisibleSelection::adjustSelectionToAvoidCrossingShadowBoundaries):
+        * editing/VisibleSelection.h:
+        (VisibleSelection):
+
 2012-06-18  Mike Lawther  <[email protected]>
 
         Crash when setting title dynamically

Modified: trunk/Source/WebCore/editing/VisibleSelection.cpp (120665 => 120666)


--- trunk/Source/WebCore/editing/VisibleSelection.cpp	2012-06-19 03:54:51 UTC (rev 120665)
+++ trunk/Source/WebCore/editing/VisibleSelection.cpp	2012-06-19 04:02:48 UTC (rev 120666)
@@ -410,28 +410,6 @@
         m_affinity = DOWNSTREAM;
 }
 
-Position VisibleSelection::adjustPositionBefore(TreeScope* treeScope, const Position& currentPosition)
-{
-    if (Node* ancestor = treeScope->ancestorInThisScope(currentPosition.anchorNode()))
-        return positionBeforeNode(ancestor);
-
-    if (Node* lastChild = treeScope->rootNode()->lastChild())
-        return positionAfterNode(lastChild);
-
-    return Position();
-}
-
-Position VisibleSelection::adjustPositionAfter(TreeScope* treeScope, const Position& currentPosition)
-{
-    if (Node* ancestor = treeScope->ancestorInThisScope(currentPosition.anchorNode()))
-        return positionAfterNode(ancestor);
-
-    if (Node* firstChild = treeScope->rootNode()->firstChild())
-        return positionBeforeNode(firstChild);
-
-    return Position();
-}
-
 void VisibleSelection::validate(TextGranularity granularity)
 {
     setBaseAndExtentToDeepEquivalents();
@@ -483,6 +461,42 @@
     m_selectionType = base == extent ? CaretSelection : RangeSelection;
 }
 
+static Position adjustPositionForEnd(const Position& currentPosition, Node* startContainerNode)
+{
+    TreeScope* treeScope = startContainerNode->treeScope();
+
+    ASSERT(currentPosition.containerNode()->treeScope() != treeScope);
+
+    if (Node* ancestor = treeScope->ancestorInThisScope(currentPosition.containerNode())) {
+        if (ancestor->contains(startContainerNode))
+            return positionAfterNode(ancestor);
+        return positionBeforeNode(ancestor);
+    }
+
+    if (Node* lastChild = treeScope->rootNode()->lastChild())
+        return positionAfterNode(lastChild);
+
+    return Position();
+}
+
+static Position adjustPositionForStart(const Position& currentPosition, Node* endContainerNode)
+{
+    TreeScope* treeScope = endContainerNode->treeScope();
+
+    ASSERT(currentPosition.containerNode()->treeScope() != treeScope);
+    
+    if (Node* ancestor = treeScope->ancestorInThisScope(currentPosition.containerNode())) {
+        if (ancestor->contains(endContainerNode))
+            return positionBeforeNode(ancestor);
+        return positionAfterNode(ancestor);
+    }
+
+    if (Node* firstChild = treeScope->rootNode()->firstChild())
+        return positionBeforeNode(firstChild);
+
+    return Position();
+}
+
 void VisibleSelection::adjustSelectionToAvoidCrossingShadowBoundaries()
 {
     if (m_base.isNull() || m_start.isNull() || m_end.isNull())
@@ -492,10 +506,10 @@
         return;
 
     if (m_baseIsFirst) {
-        m_extent = adjustPositionBefore(m_start.anchorNode()->treeScope(), m_end);
+        m_extent = adjustPositionForEnd(m_end, m_start.containerNode());
         m_end = m_extent;
     } else {
-        m_extent = adjustPositionAfter(m_end.anchorNode()->treeScope(), m_start);
+        m_extent = adjustPositionForStart(m_start, m_end.containerNode());
         m_start = m_extent;
     }
 

Modified: trunk/Source/WebCore/editing/VisibleSelection.h (120665 => 120666)


--- trunk/Source/WebCore/editing/VisibleSelection.h	2012-06-19 03:54:51 UTC (rev 120665)
+++ trunk/Source/WebCore/editing/VisibleSelection.h	2012-06-19 04:02:48 UTC (rev 120666)
@@ -122,9 +122,6 @@
     void adjustSelectionToAvoidCrossingEditingBoundaries();
     void updateSelectionType();
 
-    Position adjustPositionBefore(TreeScope*, const Position&);
-    Position adjustPositionAfter(TreeScope*, const Position&);
-
     // We need to store these as Positions because VisibleSelection is
     // used to store values in editing commands for use when
     // undoing the command. We need to be able to create a selection that, while currently
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to