- Revision
- 252647
- Author
- [email protected]
- Date
- 2019-11-19 14:05:12 -0800 (Tue, 19 Nov 2019)
Log Message
ASSERTION FAILURE: useDownstream ? (result > vp) : (result < vp) in WebCore::nextSentenceBoundaryInDirection()
https://bugs.webkit.org/show_bug.cgi?id=204370
<rdar://problem/57332559>
Reviewed by Wenson Hsieh.
Only positions whose anchor nodes are in a document and in the same tree scope can be
compared using the operator< overload. Otherwise, the result is non-deterministic by
spec. <https://dom.spec.whatwg.org/#dom-node-comparedocumentposition> (13 November 2019):
6. If node1 or node2 is null, or node1's root is not node2's root, then return the result of
adding DOCUMENT_POSITION_DISCONNECTED, DOCUMENT_POSITION_IMPLEMENTATION_SPECIFIC, and either
DOCUMENT_POSITION_PRECEDING or DOCUMENT_POSITION_FOLLOWING, with the constraint that this is
to be consistent, together.
NOTE: Whether to return DOCUMENT_POSITION_PRECEDING or DOCUMENT_POSITION_FOLLOWING is typically
implemented via pointer comparison. In _javascript_ implementations a cached Math.random()
value can be used.
* dom/Node.cpp:
(WebCore::areNodesConnectedInSameTreeScope): Added; extracted from compareDocumentPosition().
(WebCore::Node::compareDocumentPosition): Write in terms of areNodesConnectedInSameTreeScope().
* dom/Node.h:
* editing/VisiblePosition.cpp:
(WebCore::areVisiblePositionsInSameTreeScope): Added. Write in terms of areNodesConnectedInSameTreeScope().
* editing/VisiblePosition.h:
* editing/VisibleUnits.cpp:
(WebCore::nextSentenceBoundaryInDirection): Update assert. We can only compare positions
if areVisiblePositionsInSameTreeScope() returns true.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (252646 => 252647)
--- trunk/Source/WebCore/ChangeLog 2019-11-19 21:57:52 UTC (rev 252646)
+++ trunk/Source/WebCore/ChangeLog 2019-11-19 22:05:12 UTC (rev 252647)
@@ -1,3 +1,35 @@
+2019-11-19 Daniel Bates <[email protected]>
+
+ ASSERTION FAILURE: useDownstream ? (result > vp) : (result < vp) in WebCore::nextSentenceBoundaryInDirection()
+ https://bugs.webkit.org/show_bug.cgi?id=204370
+ <rdar://problem/57332559>
+
+ Reviewed by Wenson Hsieh.
+
+ Only positions whose anchor nodes are in a document and in the same tree scope can be
+ compared using the operator< overload. Otherwise, the result is non-deterministic by
+ spec. <https://dom.spec.whatwg.org/#dom-node-comparedocumentposition> (13 November 2019):
+
+ 6. If node1 or node2 is null, or node1's root is not node2's root, then return the result of
+ adding DOCUMENT_POSITION_DISCONNECTED, DOCUMENT_POSITION_IMPLEMENTATION_SPECIFIC, and either
+ DOCUMENT_POSITION_PRECEDING or DOCUMENT_POSITION_FOLLOWING, with the constraint that this is
+ to be consistent, together.
+
+ NOTE: Whether to return DOCUMENT_POSITION_PRECEDING or DOCUMENT_POSITION_FOLLOWING is typically
+ implemented via pointer comparison. In _javascript_ implementations a cached Math.random()
+ value can be used.
+
+ * dom/Node.cpp:
+ (WebCore::areNodesConnectedInSameTreeScope): Added; extracted from compareDocumentPosition().
+ (WebCore::Node::compareDocumentPosition): Write in terms of areNodesConnectedInSameTreeScope().
+ * dom/Node.h:
+ * editing/VisiblePosition.cpp:
+ (WebCore::areVisiblePositionsInSameTreeScope): Added. Write in terms of areNodesConnectedInSameTreeScope().
+ * editing/VisiblePosition.h:
+ * editing/VisibleUnits.cpp:
+ (WebCore::nextSentenceBoundaryInDirection): Update assert. We can only compare positions
+ if areVisiblePositionsInSameTreeScope() returns true.
+
2019-11-18 Ryosuke Niwa <[email protected]>
Rename AbstractEventLoop to EventLoop and move to its own cpp file
Modified: trunk/Source/WebCore/dom/Node.cpp (252646 => 252647)
--- trunk/Source/WebCore/dom/Node.cpp 2019-11-19 21:57:52 UTC (rev 252646)
+++ trunk/Source/WebCore/dom/Node.cpp 2019-11-19 22:05:12 UTC (rev 252647)
@@ -1610,6 +1610,14 @@
return Node::DOCUMENT_POSITION_DISCONNECTED | Node::DOCUMENT_POSITION_IMPLEMENTATION_SPECIFIC | direction;
}
+bool areNodesConnectedInSameTreeScope(const Node* a, const Node* b)
+{
+ if (!a || !b)
+ return false;
+ // Note that we avoid comparing Attr nodes here, since they return false from isConnected() all the time (which seems like a bug).
+ return a->isConnected() == b->isConnected() && &a->treeScope() == &b->treeScope();
+}
+
unsigned short Node::compareDocumentPosition(Node& otherNode)
{
if (&otherNode == this)
@@ -1654,9 +1662,8 @@
}
// If one node is in the document and the other is not, we must be disconnected.
- // If the nodes have different owning documents, they must be disconnected. Note that we avoid
- // comparing Attr nodes here, since they return false from isConnected() all the time (which seems like a bug).
- if (start1->isConnected() != start2->isConnected() || &start1->treeScope() != &start2->treeScope())
+ // If the nodes have different owning documents, they must be disconnected.
+ if (!areNodesConnectedInSameTreeScope(start1, start2))
return compareDetachedElementsPosition(*this, otherNode);
// We need to find a common ancestor container, and then compare the indices of the two immediate children.
Modified: trunk/Source/WebCore/dom/Node.h (252646 => 252647)
--- trunk/Source/WebCore/dom/Node.h 2019-11-19 21:57:52 UTC (rev 252646)
+++ trunk/Source/WebCore/dom/Node.h 2019-11-19 22:05:12 UTC (rev 252647)
@@ -790,6 +790,8 @@
moveTreeToNewScope(*this, *m_treeScope, newTreeScope);
}
+bool areNodesConnectedInSameTreeScope(const Node*, const Node*);
+
} // namespace WebCore
#if ENABLE(TREE_DEBUGGING)
Modified: trunk/Source/WebCore/editing/VisiblePosition.cpp (252646 => 252647)
--- trunk/Source/WebCore/editing/VisiblePosition.cpp 2019-11-19 21:57:52 UTC (rev 252646)
+++ trunk/Source/WebCore/editing/VisiblePosition.cpp 2019-11-19 22:05:12 UTC (rev 252647)
@@ -788,6 +788,11 @@
return next.isNull() || !next.deepEquivalent().deprecatedNode()->isDescendantOf(node);
}
+bool areVisiblePositionsInSameTreeScope(const VisiblePosition& a, const VisiblePosition& b)
+{
+ return areNodesConnectedInSameTreeScope(a.deepEquivalent().anchorNode(), b.deepEquivalent().anchorNode());
+}
+
bool VisiblePosition::equals(const VisiblePosition& other) const
{
return m_affinity == other.m_affinity && m_deepPosition.equals(other.m_deepPosition);
Modified: trunk/Source/WebCore/editing/VisiblePosition.h (252646 => 252647)
--- trunk/Source/WebCore/editing/VisiblePosition.h 2019-11-19 21:57:52 UTC (rev 252646)
+++ trunk/Source/WebCore/editing/VisiblePosition.h 2019-11-19 22:05:12 UTC (rev 252647)
@@ -165,6 +165,8 @@
bool isFirstVisiblePositionInNode(const VisiblePosition&, const Node*);
bool isLastVisiblePositionInNode(const VisiblePosition&, const Node*);
+bool areVisiblePositionsInSameTreeScope(const VisiblePosition&, const VisiblePosition&);
+
WTF::TextStream& operator<<(WTF::TextStream&, EAffinity);
WEBCORE_EXPORT WTF::TextStream& operator<<(WTF::TextStream&, const VisiblePosition&);
Modified: trunk/Source/WebCore/editing/VisibleUnits.cpp (252646 => 252647)
--- trunk/Source/WebCore/editing/VisibleUnits.cpp 2019-11-19 21:57:52 UTC (rev 252646)
+++ trunk/Source/WebCore/editing/VisibleUnits.cpp 2019-11-19 22:05:12 UTC (rev 252647)
@@ -1776,7 +1776,8 @@
if (result == vp)
return VisiblePosition();
- ASSERT(useDownstream ? (result > vp) : (result < vp));
+ // Positions can only be compared if they are in the same tree scope.
+ ASSERT_IMPLIES(areVisiblePositionsInSameTreeScope(result, vp), useDownstream ? (result > vp) : (result < vp));
return result;
}
