- Revision
- 238693
- Author
- rn...@webkit.org
- Date
- 2018-11-29 13:50:43 -0800 (Thu, 29 Nov 2018)
Log Message
Executing "insertunorderedlist" while selecting a contenteditable element inside a shadow dom hangs the browser
https://bugs.webkit.org/show_bug.cgi?id=184049
<rdar://problem/38931033>
Reviewed by Antti Koivisto.
Source/WebCore:
The primary hung was caused by TextIterator::advance traversing the next node in the tree order using
NodeTraversal::next which doesn't take the composed tree into account. Fixed the bug by traversing
the composed tree while sharing code with StylizedMarkupAccumulator.
This revealed a second hang in InsertListCommand::doApply() caused by endingSelection() being null,
which was caused by CompositeEditCommand::moveParagraphs failing to restore the ending selection properly
because it was computing the text indices difference from the beginning of the document until the destination.
Fixed this second bug by computing the indices against the beginning of the root editable element.
Note that editability never crosses a shadow boundary.
Test: editing/execCommand/insert-unordered-list-in-shadow-tree.html
* dom/ComposedTreeIterator.h:
(WebCore::nextSkippingChildrenInComposedTreeIgnoringUserAgentShadow): Extracted out of nextSkippingChildren
in markup.cpp.
(WebCore::nextInComposedTreeIgnoringUserAgentShadow): Added.
* editing/CompositeEditCommand.cpp:
(WebCore::CompositeEditCommand::moveParagraphs): Compute the index from the beginning of the root editable
element as opposed to the beginning of the document.
* editing/TextIterator.cpp:
(WebCore::nextNode): Added.
(WebCore::isDescendantOf): Added.
(WebCore::TextIterator::advance): Use the newly added functions to traverse the composed tree when the options
contains TextIteratorTraversesFlatTree.
* editing/markup.cpp:
LayoutTests:
Added a regression test for executing InsertUnorderedList inside a shadow tree.
* editing/execCommand/insert-ordered-list-in-shadow-tree-expected.txt: Added.
* editing/execCommand/insert-ordered-list-in-shadow-tree.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (238692 => 238693)
--- trunk/LayoutTests/ChangeLog 2018-11-29 21:26:28 UTC (rev 238692)
+++ trunk/LayoutTests/ChangeLog 2018-11-29 21:50:43 UTC (rev 238693)
@@ -1,3 +1,16 @@
+2018-11-29 Ryosuke Niwa <rn...@webkit.org>
+
+ Executing "insertunorderedlist" while selecting a contenteditable element inside a shadow dom hangs the browser
+ https://bugs.webkit.org/show_bug.cgi?id=184049
+ <rdar://problem/38931033>
+
+ Reviewed by Antti Koivisto.
+
+ Added a regression test for executing InsertUnorderedList inside a shadow tree.
+
+ * editing/execCommand/insert-ordered-list-in-shadow-tree-expected.txt: Added.
+ * editing/execCommand/insert-ordered-list-in-shadow-tree.html: Added.
+
2018-11-29 Justin Fan <justin_...@apple.com>
[WebGPU] WebGPURenderPassEncoder::setPipeline, draw, and endPass prototypes
Added: trunk/LayoutTests/editing/execCommand/insert-unordered-list-in-shadow-tree-expected.txt (0 => 238693)
--- trunk/LayoutTests/editing/execCommand/insert-unordered-list-in-shadow-tree-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/execCommand/insert-unordered-list-in-shadow-tree-expected.txt 2018-11-29 21:50:43 UTC (rev 238693)
@@ -0,0 +1,28 @@
+This tests inserting an ordered list inside a shadow tree. WebKit should not hang.
+
+Before:
+| <div>
+| contenteditable=""
+| id="editor"
+| "
+ one"
+| <br>
+| "
+ two"
+| <br>
+| "
+"
+
+After:
+| <div>
+| contenteditable=""
+| id="editor"
+| <ol>
+| <li>
+| "one"
+| <br>
+| <li>
+| "two"
+| <br>
+| "
+"
Added: trunk/LayoutTests/editing/execCommand/insert-unordered-list-in-shadow-tree.html (0 => 238693)
--- trunk/LayoutTests/editing/execCommand/insert-unordered-list-in-shadow-tree.html (rev 0)
+++ trunk/LayoutTests/editing/execCommand/insert-unordered-list-in-shadow-tree.html 2018-11-29 21:50:43 UTC (rev 238693)
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script>
+
+Markup.description('This tests inserting an ordered list inside a shadow tree. WebKit should not hang.');
+
+const host = document.createElement('div');
+document.body.appendChild(host);
+const shadowRoot = host.attachShadow({mode: 'closed'});
+shadowRoot.innerHTML = `<div id="editor" contenteditable>
+ one<br>
+ two<br>
+</div>`;
+
+shadowRoot.querySelector('#editor').focus();
+document.execCommand('selectAll', false, null);
+Markup.dump(shadowRoot, 'Before');
+
+document.execCommand('insertOrderedList', false, null);
+Markup.dump(shadowRoot, 'After');
+
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (238692 => 238693)
--- trunk/Source/WebCore/ChangeLog 2018-11-29 21:26:28 UTC (rev 238692)
+++ trunk/Source/WebCore/ChangeLog 2018-11-29 21:50:43 UTC (rev 238693)
@@ -1,3 +1,38 @@
+2018-11-29 Ryosuke Niwa <rn...@webkit.org>
+
+ Executing "insertunorderedlist" while selecting a contenteditable element inside a shadow dom hangs the browser
+ https://bugs.webkit.org/show_bug.cgi?id=184049
+ <rdar://problem/38931033>
+
+ Reviewed by Antti Koivisto.
+
+ The primary hung was caused by TextIterator::advance traversing the next node in the tree order using
+ NodeTraversal::next which doesn't take the composed tree into account. Fixed the bug by traversing
+ the composed tree while sharing code with StylizedMarkupAccumulator.
+
+ This revealed a second hang in InsertListCommand::doApply() caused by endingSelection() being null,
+ which was caused by CompositeEditCommand::moveParagraphs failing to restore the ending selection properly
+ because it was computing the text indices difference from the beginning of the document until the destination.
+
+ Fixed this second bug by computing the indices against the beginning of the root editable element.
+ Note that editability never crosses a shadow boundary.
+
+ Test: editing/execCommand/insert-unordered-list-in-shadow-tree.html
+
+ * dom/ComposedTreeIterator.h:
+ (WebCore::nextSkippingChildrenInComposedTreeIgnoringUserAgentShadow): Extracted out of nextSkippingChildren
+ in markup.cpp.
+ (WebCore::nextInComposedTreeIgnoringUserAgentShadow): Added.
+ * editing/CompositeEditCommand.cpp:
+ (WebCore::CompositeEditCommand::moveParagraphs): Compute the index from the beginning of the root editable
+ element as opposed to the beginning of the document.
+ * editing/TextIterator.cpp:
+ (WebCore::nextNode): Added.
+ (WebCore::isDescendantOf): Added.
+ (WebCore::TextIterator::advance): Use the newly added functions to traverse the composed tree when the options
+ contains TextIteratorTraversesFlatTree.
+ * editing/markup.cpp:
+
2018-11-29 Zalan Bujtas <za...@apple.com>
[ContentObservation] Decouple content change and DOM timer scheduling observation
Modified: trunk/Source/WebCore/dom/ComposedTreeIterator.h (238692 => 238693)
--- trunk/Source/WebCore/dom/ComposedTreeIterator.h 2018-11-29 21:26:28 UTC (rev 238692)
+++ trunk/Source/WebCore/dom/ComposedTreeIterator.h 2018-11-29 21:50:43 UTC (rev 238693)
@@ -245,4 +245,22 @@
return node.nextSibling();
}
+inline Node* nextSkippingChildrenInComposedTreeIgnoringUserAgentShadow(Node& node)
+{
+ if (auto* sibling = nextSiblingInComposedTreeIgnoringUserAgentShadow(node))
+ return sibling;
+ for (auto* ancestor = node.parentInComposedTree(); ancestor; ancestor = ancestor->parentInComposedTree()) {
+ if (auto* sibling = nextSiblingInComposedTreeIgnoringUserAgentShadow(*ancestor))
+ return sibling;
+ }
+ return nullptr;
+}
+
+inline Node* nextInComposedTreeIgnoringUserAgentShadow(Node& node)
+{
+ if (auto* firstChild = firstChildInComposedTreeIgnoringUserAgentShadow(node))
+ return firstChild;
+ return nextSkippingChildrenInComposedTreeIgnoringUserAgentShadow(node);
+}
+
} // namespace WebCore
Modified: trunk/Source/WebCore/editing/CompositeEditCommand.cpp (238692 => 238693)
--- trunk/Source/WebCore/editing/CompositeEditCommand.cpp 2018-11-29 21:26:28 UTC (rev 238692)
+++ trunk/Source/WebCore/editing/CompositeEditCommand.cpp 2018-11-29 21:50:43 UTC (rev 238693)
@@ -1487,7 +1487,8 @@
document().updateLayoutIgnorePendingStylesheets();
}
- RefPtr<Range> startToDestinationRange(Range::create(document(), firstPositionInNode(document().documentElement()), destination.deepEquivalent().parentAnchoredEquivalent()));
+ auto editableRoot = destination.rootEditableElement();
+ RefPtr<Range> startToDestinationRange(Range::create(document(), firstPositionInNode(editableRoot), destination.deepEquivalent().parentAnchoredEquivalent()));
destinationIndex = TextIterator::rangeLength(startToDestinationRange.get(), true);
setEndingSelection(VisibleSelection(destination, originalIsDirectional));
@@ -1510,8 +1511,8 @@
// causes spaces to be collapsed during the move operation. This results
// in a call to rangeFromLocationAndLength with a location past the end
// of the document (which will return null).
- RefPtr<Range> start = TextIterator::rangeFromLocationAndLength(document().documentElement(), destinationIndex + startIndex, 0, true);
- RefPtr<Range> end = TextIterator::rangeFromLocationAndLength(document().documentElement(), destinationIndex + endIndex, 0, true);
+ RefPtr<Range> start = TextIterator::rangeFromLocationAndLength(editableRoot, destinationIndex + startIndex, 0, true);
+ RefPtr<Range> end = TextIterator::rangeFromLocationAndLength(editableRoot, destinationIndex + endIndex, 0, true);
if (start && end)
setEndingSelection(VisibleSelection(start->startPosition(), end->startPosition(), DOWNSTREAM, originalIsDirectional));
}
Modified: trunk/Source/WebCore/editing/TextIterator.cpp (238692 => 238693)
--- trunk/Source/WebCore/editing/TextIterator.cpp 2018-11-29 21:26:28 UTC (rev 238692)
+++ trunk/Source/WebCore/editing/TextIterator.cpp 2018-11-29 21:50:43 UTC (rev 238693)
@@ -424,6 +424,20 @@
return node.nextSibling();
}
+static inline Node* nextNode(TextIteratorBehavior options, Node& node)
+{
+ if (UNLIKELY(options & TextIteratorTraversesFlatTree))
+ return nextInComposedTreeIgnoringUserAgentShadow(node);
+ return NodeTraversal::next(node);
+}
+
+static inline bool isDescendantOf(TextIteratorBehavior options, Node& node, Node& possibleAncestor)
+{
+ if (UNLIKELY(options & TextIteratorTraversesFlatTree))
+ return node.isDescendantOrShadowDescendantOf(&possibleAncestor);
+ return node.isDescendantOf(&possibleAncestor);
+}
+
static inline Node* parentNodeOrShadowHost(TextIteratorBehavior options, Node& node)
{
if (UNLIKELY(options & TextIteratorTraversesFlatTree))
@@ -507,10 +521,10 @@
if (!next) {
next = nextSibling(m_behavior, *m_node);
if (!next) {
- bool pastEnd = NodeTraversal::next(*m_node) == m_pastEndNode;
+ bool pastEnd = nextNode(m_behavior, *m_node) == m_pastEndNode;
Node* parentNode = parentNodeOrShadowHost(m_behavior, *m_node);
while (!next && parentNode) {
- if ((pastEnd && parentNode == m_endContainer) || m_endContainer->isDescendantOf(*parentNode))
+ if ((pastEnd && parentNode == m_endContainer) || isDescendantOf(m_behavior, *m_endContainer, *parentNode))
return;
bool haveRenderer = m_node->renderer();
Node* exitedNode = m_node;
Modified: trunk/Source/WebCore/editing/markup.cpp (238692 => 238693)
--- trunk/Source/WebCore/editing/markup.cpp 2018-11-29 21:26:28 UTC (rev 238692)
+++ trunk/Source/WebCore/editing/markup.cpp 2018-11-29 21:50:43 UTC (rev 238693)
@@ -273,15 +273,8 @@
Node* nextSkippingChildren(Node& node)
{
- if (UNLIKELY(m_useComposedTree)) {
- if (auto* sibling = nextSiblingInComposedTreeIgnoringUserAgentShadow(node))
- return sibling;
- for (auto* ancestor = node.parentInComposedTree(); ancestor; ancestor = ancestor->parentInComposedTree()) {
- if (auto* sibling = nextSiblingInComposedTreeIgnoringUserAgentShadow(*ancestor))
- return sibling;
- }
- return nullptr;
- }
+ if (UNLIKELY(m_useComposedTree))
+ return nextSkippingChildrenInComposedTreeIgnoringUserAgentShadow(node);
return NodeTraversal::nextSkippingChildren(node);
}
