Title: [274626] trunk/Source/WebCore
- Revision
- 274626
- Author
- commit-qu...@webkit.org
- Date
- 2021-03-18 02:18:40 -0700 (Thu, 18 Mar 2021)
Log Message
ASSERTION FAILED: node.isConnected() in matchSlottedPseudoElementRules
https://bugs.webkit.org/show_bug.cgi?id=221440
Patch by Frédéric Wang <fw...@igalia.com> on 2021-03-18
Reviewed by Ryosuke Niwa.
ReplaceSelectionCommand::doApply() removes a <br> from an element and immediately calls
highestNodeToRemoveInPruning() on that element. The former operation may destroy the
element's renderer and confuses the latter operation. This happens in particular for a
<summary> element which ends up being removed from the tree. This in turn causes unexpected
issues such as a debug assertion failure in matchSlottedPseudoElementRules. To address that
problem, ensure the document is laid out before calling highestNodeToRemoveInPruning().
This patch also increases and improves use of RefPtr<Node>.
* editing/CompositeEditCommand.cpp:
(WebCore::CompositeEditCommand::removeNodeAndPruneAncestors): Use auto & makeRefPtr.
(WebCore::CompositeEditCommand::prune): Store local highestNodeToRemove variable in a RefPtr.
(WebCore::CompositeEditCommand::cleanupAfterDeletion): Store local node variable in a RefPtr.
(WebCore::CompositeEditCommand::breakOutOfEmptyMailBlockquotedParagraph): Store local parentNode variable in a RefPtr.
* editing/Editing.cpp:
(WebCore::highestNodeToRemoveInPruning): Store local currentNode variable in a a RefPtr.
* editing/ReplaceSelectionCommand.cpp:
(WebCore::ReplaceSelectionCommand::doApply): Use auto & makeRefPtr. Store local odeToRemove variable in a RefPtr.
Ensure the document is laid out before calling highestNodeToRemoveInPruning.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (274625 => 274626)
--- trunk/Source/WebCore/ChangeLog 2021-03-18 09:11:13 UTC (rev 274625)
+++ trunk/Source/WebCore/ChangeLog 2021-03-18 09:18:40 UTC (rev 274626)
@@ -1,3 +1,29 @@
+2021-03-18 Frédéric Wang <fw...@igalia.com>
+
+ ASSERTION FAILED: node.isConnected() in matchSlottedPseudoElementRules
+ https://bugs.webkit.org/show_bug.cgi?id=221440
+
+ Reviewed by Ryosuke Niwa.
+
+ ReplaceSelectionCommand::doApply() removes a <br> from an element and immediately calls
+ highestNodeToRemoveInPruning() on that element. The former operation may destroy the
+ element's renderer and confuses the latter operation. This happens in particular for a
+ <summary> element which ends up being removed from the tree. This in turn causes unexpected
+ issues such as a debug assertion failure in matchSlottedPseudoElementRules. To address that
+ problem, ensure the document is laid out before calling highestNodeToRemoveInPruning().
+ This patch also increases and improves use of RefPtr<Node>.
+
+ * editing/CompositeEditCommand.cpp:
+ (WebCore::CompositeEditCommand::removeNodeAndPruneAncestors): Use auto & makeRefPtr.
+ (WebCore::CompositeEditCommand::prune): Store local highestNodeToRemove variable in a RefPtr.
+ (WebCore::CompositeEditCommand::cleanupAfterDeletion): Store local node variable in a RefPtr.
+ (WebCore::CompositeEditCommand::breakOutOfEmptyMailBlockquotedParagraph): Store local parentNode variable in a RefPtr.
+ * editing/Editing.cpp:
+ (WebCore::highestNodeToRemoveInPruning): Store local currentNode variable in a a RefPtr.
+ * editing/ReplaceSelectionCommand.cpp:
+ (WebCore::ReplaceSelectionCommand::doApply): Use auto & makeRefPtr. Store local odeToRemove variable in a RefPtr.
+ Ensure the document is laid out before calling highestNodeToRemoveInPruning.
+
2021-03-18 Devin Rousso <drou...@apple.com>
should always use all prerequisites for the `ModernMediaControls.{js,css}` rules in DerivedSources.make
Modified: trunk/Source/WebCore/editing/CompositeEditCommand.cpp (274625 => 274626)
--- trunk/Source/WebCore/editing/CompositeEditCommand.cpp 2021-03-18 09:11:13 UTC (rev 274625)
+++ trunk/Source/WebCore/editing/CompositeEditCommand.cpp 2021-03-18 09:18:40 UTC (rev 274626)
@@ -612,7 +612,7 @@
void CompositeEditCommand::removeNodeAndPruneAncestors(Node& node)
{
- RefPtr<ContainerNode> parent = node.parentNode();
+ auto parent = makeRefPtr(node.parentNode());
removeNode(node);
prune(parent.get());
}
@@ -656,7 +656,7 @@
void CompositeEditCommand::prune(Node* node)
{
- if (auto* highestNodeToRemove = highestNodeToRemoveInPruning(node))
+ if (auto highestNodeToRemove = makeRefPtr(highestNodeToRemoveInPruning(node)))
removeNode(*highestNodeToRemove);
}
@@ -1297,7 +1297,7 @@
if (!caretAfterDelete.equals(destination) && isStartOfParagraph(caretAfterDelete) && isEndOfParagraph(caretAfterDelete)) {
// Note: We want the rightmost candidate.
Position position = caretAfterDelete.deepEquivalent().downstream();
- Node* node = position.deprecatedNode();
+ auto node = makeRefPtr(position.deprecatedNode());
ASSERT(node);
// Normally deletion will leave a br as a placeholder.
if (is<HTMLBRElement>(*node))
@@ -1306,11 +1306,11 @@
// doesn't require a placeholder to prop itself open (like a bordered
// div or an li), remove it during the move (the list removal code
// expects this behavior).
- else if (isBlock(node)) {
+ else if (isBlock(node.get())) {
// If caret position after deletion and destination position coincides,
// node should not be removed.
if (!position.rendersInDifferentPosition(destination.deepEquivalent())) {
- prune(node);
+ prune(node.get());
return;
}
removeNodeAndPruneAncestors(*node);
@@ -1627,11 +1627,11 @@
else if (is<Text>(*caretPos.deprecatedNode())) {
ASSERT(caretPos.deprecatedEditingOffset() == 0);
Text& textNode = downcast<Text>(*caretPos.deprecatedNode());
- ContainerNode* parentNode = textNode.parentNode();
+ auto parentNode = makeRefPtr(textNode.parentNode());
// The preserved newline must be the first thing in the node, since otherwise the previous
// paragraph would be quoted, and we verified that it wasn't above.
deleteTextFromNode(textNode, 0, 1);
- prune(parentNode);
+ prune(parentNode.get());
}
return true;
Modified: trunk/Source/WebCore/editing/Editing.cpp (274625 => 274626)
--- trunk/Source/WebCore/editing/Editing.cpp 2021-03-18 09:11:13 UTC (rev 274625)
+++ trunk/Source/WebCore/editing/Editing.cpp 2021-03-18 09:18:40 UTC (rev 274626)
@@ -628,12 +628,12 @@
{
Node* previousNode = nullptr;
auto* rootEditableElement = node ? node->rootEditableElement() : nullptr;
- for (; node; node = node->parentNode()) {
- if (auto* renderer = node->renderer()) {
- if (!renderer->canHaveChildren() || hasARenderedDescendant(node, previousNode) || rootEditableElement == node)
+ for (auto currentNode = makeRefPtr(node); currentNode; currentNode = currentNode->parentNode()) {
+ if (auto* renderer = currentNode->renderer()) {
+ if (!renderer->canHaveChildren() || hasARenderedDescendant(currentNode.get(), previousNode) || rootEditableElement == currentNode.get())
return previousNode;
}
- previousNode = node;
+ previousNode = currentNode.get();
}
return nullptr;
}
Modified: trunk/Source/WebCore/editing/ReplaceSelectionCommand.cpp (274625 => 274626)
--- trunk/Source/WebCore/editing/ReplaceSelectionCommand.cpp 2021-03-18 09:11:13 UTC (rev 274625)
+++ trunk/Source/WebCore/editing/ReplaceSelectionCommand.cpp 2021-03-18 09:18:40 UTC (rev 274626)
@@ -1297,11 +1297,12 @@
insertNodeAt(HTMLBRElement::create(document()), startOfInsertedContent.deepEquivalent());
if (endBR && (plainTextFragment || shouldRemoveEndBR(endBR.get(), originalVisPosBeforeEndBR))) {
- RefPtr<Node> parent = endBR->parentNode();
+ auto parent = makeRefPtr(endBR->parentNode());
insertedNodes.willRemoveNode(endBR.get());
removeNode(*endBR);
- if (Node* nodeToRemove = highestNodeToRemoveInPruning(parent.get())) {
- insertedNodes.willRemoveNode(nodeToRemove);
+ document().updateLayoutIgnorePendingStylesheets();
+ if (auto nodeToRemove = makeRefPtr(highestNodeToRemoveInPruning(parent.get()))) {
+ insertedNodes.willRemoveNode(nodeToRemove.get());
removeNode(*nodeToRemove);
}
}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
