Skip to content

Commit

Permalink
Cherry-pick 259548.273@safari-7615-branch (0501c9f). https://bugs.web…
Browse files Browse the repository at this point in the history
…kit.org/show_bug.cgi?id=252785

    Assertion failure in ContainerNode::removeAllChildrenWithScriptAssertion
    https://bugs.webkit.org/show_bug.cgi?id=252785
    rdar://105643144

    Reviewed by Chris Dumez and Geoffrey Garen.

    The bug was caused by Editor::changeSelectionAfterCommand setting selection pointing to a wrong document.
    Fixed the bug by detecting this case and exiting early.

    * LayoutTests/editing/selection/redo-selection-restore-different-document-crash-expected.txt: Added.
    * LayoutTests/editing/selection/redo-selection-restore-different-document-crash.html: Added.
    * Source/WebCore/editing/Editor.cpp:
    (WebCore::Editor::changeSelectionAfterCommand):
    * Source/WebCore/editing/VisibleSelection.cpp:
    (WebCore::VisibleSelection::document const): Now returns the document of m_anchor when base is null but anchor isn't.

    Canonical link: https://commits.webkit.org/259548.273@safari-7615-branch
  • Loading branch information
rniwa authored and aperezdc committed Apr 3, 2023
1 parent 1c1cec3 commit db99b3e
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 7 deletions.
@@ -0,0 +1 @@
PASS
@@ -0,0 +1,30 @@
<script>
if (window.testRunner)
testRunner.dumpAsText();
function main() {
var input1 = document.getElementById("input1");
input1.selectionDirection = "backward";
document.execCommand("insertText", false, "AA");
x1.show();
input1.defaultValue = ""; // 4
input2.setRangeText("A"); // 5
document.body.textContent = 'PASS';
}

function f1() {
document.execCommand("undo",false,null); // 0, insertText "AA"
document.execCommand("redo",false,null); // 3
}

function f2() {
parser = new DOMParser();
doc = parser.parseFromString("A", "text/html");
input1.defaultValue = "A"; // 1
doc.body.appendChild(input1); // 2
}
</script>
<body onload="main()">
<dialog id="x1" onfocusin="f1()" onfocusout="f2()" tabindex="-1">
</dialog>
<input id="input1" style="display: contents">
<input id="input2" value="A">
4 changes: 3 additions & 1 deletion Source/WebCore/editing/Editor.cpp
Expand Up @@ -3350,7 +3350,7 @@ void Editor::changeSelectionAfterCommand(const VisibleSelection& newSelection, O
{
Ref<Document> protectedDocument(m_document);

if (newSelection.isOrphan())
if (newSelection.isOrphan() || newSelection.document() != protectedDocument.ptr())
return;

// If there is no selection change, don't bother sending shouldChangeSelection, but still call setSelection,
Expand All @@ -3361,6 +3361,8 @@ void Editor::changeSelectionAfterCommand(const VisibleSelection& newSelection, O
if (selectionDidNotChangeDOMPosition || m_document.selection().shouldChangeSelection(newSelection))
m_document.selection().setSelection(newSelection, options);

ASSERT_WITH_SECURITY_IMPLICATION(!m_document.selection().selection().isOrphan());

// Some editing operations change the selection visually without affecting its position within the DOM.
// For example when you press return in the following (the caret is marked by ^):
// <div contentEditable="true"><div>^Hello</div></div>
Expand Down
15 changes: 9 additions & 6 deletions Source/WebCore/editing/VisibleSelection.cpp
Expand Up @@ -152,17 +152,20 @@ bool VisibleSelection::isOrphan() const

RefPtr<Document> VisibleSelection::document() const
{
RefPtr baseDocument { m_base.document() };
if (!baseDocument)
return nullptr;
RefPtr document { m_base.document() };
if (!document) {
document = m_anchor.document();
if (!document || !document->settings().liveRangeSelectionEnabled())
return nullptr;
}

if (m_extent.document() != baseDocument.get() || m_start.document() != baseDocument.get() || m_end.document() != baseDocument.get())
if (m_extent.document() != document.get() || m_start.document() != document.get() || m_end.document() != document.get())
return nullptr;

if (baseDocument->settings().liveRangeSelectionEnabled() && (m_anchor.document() != baseDocument.get() || m_focus.document() != baseDocument.get()))
if (document->settings().liveRangeSelectionEnabled() && (m_anchor.document() != document.get() || m_focus.document() != document.get()))
return nullptr;

return baseDocument;
return document;
}

std::optional<SimpleRange> VisibleSelection::firstRange() const
Expand Down

0 comments on commit db99b3e

Please sign in to comment.