Diff
Modified: trunk/LayoutTests/ChangeLog (201666 => 201667)
--- trunk/LayoutTests/ChangeLog 2016-06-03 22:47:17 UTC (rev 201666)
+++ trunk/LayoutTests/ChangeLog 2016-06-03 23:01:49 UTC (rev 201667)
@@ -1,3 +1,15 @@
+2016-06-03 Ryosuke Niwa <[email protected]>
+
+ Crash under VisibleSelection::firstRange()
+ https://bugs.webkit.org/show_bug.cgi?id=158241
+
+ Reviewed by Enrica Casucci.
+
+ Added a regression test.
+
+ * fast/shadow-dom/selection-at-shadow-root-crash-expected.txt: Added.
+ * fast/shadow-dom/selection-at-shadow-root-crash.html: Added.
+
2016-06-03 Zalan Bujtas <[email protected]>
Incorrect rendering on boostmobile FAQ page
Added: trunk/LayoutTests/fast/shadow-dom/selection-at-shadow-root-crash-expected.txt (0 => 201667)
--- trunk/LayoutTests/fast/shadow-dom/selection-at-shadow-root-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/shadow-dom/selection-at-shadow-root-crash-expected.txt 2016-06-03 23:01:49 UTC (rev 201667)
@@ -0,0 +1,4 @@
+This tests copying an image which is a direct child of a shadow root. To manually test, copy the image by pressing command / control + c. WebKit should not crash or hit an assertion.
+
+PASS - WebKit did not crash
+
Added: trunk/LayoutTests/fast/shadow-dom/selection-at-shadow-root-crash.html (0 => 201667)
--- trunk/LayoutTests/fast/shadow-dom/selection-at-shadow-root-crash.html (rev 0)
+++ trunk/LayoutTests/fast/shadow-dom/selection-at-shadow-root-crash.html 2016-06-03 23:01:49 UTC (rev 201667)
@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+<body>
+<p>This tests copying an image which is a direct child of a shadow root.
+To manually test, copy the image by pressing command / control + c. WebKit should not crash or hit an assertion.
+</p>
+<pre id="result"></pre>
+<script>
+if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+}
+
+var host = document.createElement('div');
+var root = host.attachShadow({mode: 'closed'});
+root.innerHTML = '<img src="" _onload_="runTest()">';
+
+document.body.appendChild(host);
+
+function runTest() {
+ window.getSelection().selectAllChildren(root);
+ document.execCommand('copy', null, false);
+ document.getElementById('result').textContent = 'PASS - WebKit did not crash';
+
+ if (testRunner)
+ testRunner.notifyDone();
+}
+
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (201666 => 201667)
--- trunk/Source/WebCore/ChangeLog 2016-06-03 22:47:17 UTC (rev 201666)
+++ trunk/Source/WebCore/ChangeLog 2016-06-03 23:01:49 UTC (rev 201667)
@@ -1,3 +1,41 @@
+2016-06-03 Ryosuke Niwa <[email protected]>
+
+ Crash under VisibleSelection::firstRange()
+ https://bugs.webkit.org/show_bug.cgi?id=158241
+
+ Reviewed by Enrica Casucci.
+
+ The crash was commonly caused by parentAnchoredEquivalent returning null when the anchored node was a shadow root.
+ Fixed it by returning a shadow root in parentAnchoredEquivalent.
+
+ Also guard against other kinds of crashes by adding a null check in VisibleSelection::firstRange() since we've seen
+ a crash in the same code path outside of a shadow tree.
+
+ This patch also fixes other Position methods to stop using nonShadowBoundaryParentNode in place of parentNode as
+ that would cause a similar crash and/or a bug elsewhere.
+
+ Test: fast/shadow-dom/selection-at-shadow-root-crash.html
+
+ * accessibility/AXObjectCache.cpp:
+ (AXObjectCache::startCharacterOffsetOfParagraph): Fixed a bug uncovered by the assertion fix in Position::Position.
+ This code was sometimes creating a position inside a BR, which is wrong.
+ (AXObjectCache::endCharacterOffsetOfParagraph): Ditto.
+ * dom/Position.cpp:
+ (WebCore::Position::Position): Fixed an assertion which was checking that this constructor wasn't being called
+ with m_anchorNode set to an element editing ignores content of. ||ing it with isShadowRoot() made this assertion
+ useless because it's true whenever m_anchorNode is not a shadow root.
+ (WebCore::Position::containerNode): Use parentNode() instead of findParent() which calls nonShadowBoundaryParentNode
+ since Position should
+ (WebCore::Position::parentAnchoredEquivalent): Fixed the bug by letting this function return a shadow root.
+ (WebCore::Position::previous): Use parentNode() instead of findParent().
+ (WebCore::Position::next): Ditto.
+ (WebCore::Position::atStartOfTree): Ditto.
+ (WebCore::Position::atEndOfTree): Ditto.
+ (WebCore::Position::findParent): Deleted.
+ * dom/Position.h:
+ * editing/VisibleSelection.cpp:
+ (VisibleSelection::firstRange): Added a null check.
+
2016-06-03 Zalan Bujtas <[email protected]>
Incorrect rendering on boostmobile FAQ page
Modified: trunk/Source/WebCore/accessibility/AXObjectCache.cpp (201666 => 201667)
--- trunk/Source/WebCore/accessibility/AXObjectCache.cpp 2016-06-03 22:47:17 UTC (rev 201666)
+++ trunk/Source/WebCore/accessibility/AXObjectCache.cpp 2016-06-03 23:01:49 UTC (rev 201667)
@@ -2329,8 +2329,7 @@
auto* startBlock = enclosingBlock(startNode);
int offset = characterOffset.startIndex + characterOffset.offset;
- Position p(startNode, offset, Position::PositionIsOffsetInAnchor);
- auto* highestRoot = highestEditableRoot(p);
+ auto* highestRoot = highestEditableRoot(firstPositionInOrBeforeNode(startNode));
Position::AnchorType type = Position::PositionIsOffsetInAnchor;
auto* node = findStartOfParagraph(startNode, highestRoot, startBlock, offset, type, boundaryCrossingRule);
@@ -2352,8 +2351,7 @@
Node* stayInsideBlock = enclosingBlock(startNode);
int offset = characterOffset.startIndex + characterOffset.offset;
- Position p(startNode, offset, Position::PositionIsOffsetInAnchor);
- Node* highestRoot = highestEditableRoot(p);
+ Node* highestRoot = highestEditableRoot(firstPositionInOrBeforeNode(startNode));
Position::AnchorType type = Position::PositionIsOffsetInAnchor;
Node* node = findEndOfParagraph(startNode, highestRoot, stayInsideBlock, offset, type, boundaryCrossingRule);
Modified: trunk/Source/WebCore/dom/Position.cpp (201666 => 201667)
--- trunk/Source/WebCore/dom/Position.cpp 2016-06-03 22:47:17 UTC (rev 201666)
+++ trunk/Source/WebCore/dom/Position.cpp 2016-06-03 23:01:49 UTC (rev 201667)
@@ -127,7 +127,7 @@
, m_anchorType(anchorType)
, m_isLegacyEditingPosition(false)
{
- ASSERT(!m_anchorNode || !editingIgnoresContent(*m_anchorNode) || !m_anchorNode->isShadowRoot());
+ ASSERT(!m_anchorNode || !editingIgnoresContent(*m_anchorNode));
ASSERT(!m_anchorNode || !m_anchorNode->isPseudoElement());
ASSERT(anchorType == PositionIsOffsetInAnchor);
}
@@ -170,7 +170,7 @@
return m_anchorNode.get();
case PositionIsBeforeAnchor:
case PositionIsAfterAnchor:
- return findParent(*m_anchorNode);
+ return m_anchorNode->parentNode();
}
ASSERT_NOT_REACHED();
return nullptr;
@@ -231,7 +231,7 @@
// FIXME: This should only be necessary for legacy positions, but is also needed for positions before and after Tables
if (m_offset <= 0 && (m_anchorType != PositionIsAfterAnchor && m_anchorType != PositionIsAfterChildren)) {
- if (findParent(*m_anchorNode) && (editingIgnoresContent(*m_anchorNode) || isRenderedTable(m_anchorNode.get())))
+ if (m_anchorNode->parentNode() && (editingIgnoresContent(*m_anchorNode) || isRenderedTable(m_anchorNode.get())))
return positionInParentBeforeNode(m_anchorNode.get());
return Position(m_anchorNode.get(), 0, PositionIsOffsetInAnchor);
}
@@ -344,7 +344,7 @@
}
}
- ContainerNode* parent = findParent(*node);
+ ContainerNode* parent = node->parentNode();
if (!parent)
return *this;
@@ -391,7 +391,7 @@
return createLegacyEditingPosition(node, (moveType == Character) ? uncheckedNextOffset(node, offset) : offset + 1);
}
- ContainerNode* parent = findParent(*node);
+ ContainerNode* parent = node->parentNode();
if (!parent)
return *this;
@@ -493,7 +493,7 @@
return true;
Node* container = containerNode();
- if (container && findParent(*container))
+ if (container && container->parentNode())
return false;
switch (m_anchorType) {
@@ -518,7 +518,7 @@
return true;
Node* container = containerNode();
- if (container && findParent(*container))
+ if (container && container->parentNode())
return false;
switch (m_anchorType) {
@@ -953,11 +953,6 @@
return node && node->renderer() && node->renderer()->style().userSelect() == SELECT_NONE;
}
-ContainerNode* Position::findParent(const Node& node)
-{
- return node.nonShadowBoundaryParentNode();
-}
-
#if ENABLE(USERSELECT_ALL)
bool Position::nodeIsUserSelectAll(const Node* node)
{
Modified: trunk/Source/WebCore/dom/Position.h (201666 => 201667)
--- trunk/Source/WebCore/dom/Position.h 2016-06-03 22:47:17 UTC (rev 201666)
+++ trunk/Source/WebCore/dom/Position.h 2016-06-03 23:01:49 UTC (rev 201667)
@@ -199,8 +199,7 @@
static bool nodeIsUserSelectAll(const Node*) { return false; }
static Node* rootUserSelectAllForNode(Node*) { return 0; }
#endif
- static ContainerNode* findParent(const Node&);
-
+
void debugPosition(const char* msg = "") const;
#if ENABLE(TREE_DEBUGGING)
Modified: trunk/Source/WebCore/editing/VisiblePosition.cpp (201666 => 201667)
--- trunk/Source/WebCore/editing/VisiblePosition.cpp 2016-06-03 22:47:17 UTC (rev 201666)
+++ trunk/Source/WebCore/editing/VisiblePosition.cpp 2016-06-03 23:01:49 UTC (rev 201667)
@@ -587,7 +587,7 @@
// If the html element is editable, descending into its body will look like a descent
// from non-editable to editable content since rootEditableElement() always stops at the body.
- if ((editingRoot && editingRoot->hasTagName(htmlTag)) || position.deprecatedNode()->isDocumentNode())
+ if ((editingRoot && editingRoot->hasTagName(htmlTag)) || (node && (node->isDocumentNode() || node->isShadowRoot())))
return next.isNotNull() ? next : prev;
bool prevIsInSameEditableElement = prevNode && editableRootForPosition(prev) == editingRoot;
Modified: trunk/Source/WebCore/editing/VisibleSelection.cpp (201666 => 201667)
--- trunk/Source/WebCore/editing/VisibleSelection.cpp 2016-06-03 22:47:17 UTC (rev 201666)
+++ trunk/Source/WebCore/editing/VisibleSelection.cpp 2016-06-03 23:01:49 UTC (rev 201667)
@@ -129,6 +129,8 @@
return nullptr;
Position start = m_start.parentAnchoredEquivalent();
Position end = m_end.parentAnchoredEquivalent();
+ if (start.isNull() || end.isNull())
+ return nullptr;
return Range::create(start.anchorNode()->document(), start, end);
}
