Title: [266556] trunk/Source/WebCore
Revision
266556
Author
commit-qu...@webkit.org
Date
2020-09-03 14:35:17 -0700 (Thu, 03 Sep 2020)

Log Message

Unreviewed, reverting r266498.
https://bugs.webkit.org/show_bug.cgi?id=216143

crashes in debug

Reverted changeset:

"Simplify some editing code"
https://bugs.webkit.org/show_bug.cgi?id=216097
https://trac.webkit.org/changeset/266498

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (266555 => 266556)


--- trunk/Source/WebCore/ChangeLog	2020-09-03 21:31:21 UTC (rev 266555)
+++ trunk/Source/WebCore/ChangeLog	2020-09-03 21:35:17 UTC (rev 266556)
@@ -1,3 +1,16 @@
+2020-09-03  Commit Queue  <commit-qu...@webkit.org>
+
+        Unreviewed, reverting r266498.
+        https://bugs.webkit.org/show_bug.cgi?id=216143
+
+        crashes in debug
+
+        Reverted changeset:
+
+        "Simplify some editing code"
+        https://bugs.webkit.org/show_bug.cgi?id=216097
+        https://trac.webkit.org/changeset/266498
+
 2020-09-03  Frank Yang  <guowei_y...@apple.com>
 
         CoreImage Implementation of SourceGraphic and saturate(), hue-rotate(), grayscale() and sepia()

Modified: trunk/Source/WebCore/dom/ContainerNode.cpp (266555 => 266556)


--- trunk/Source/WebCore/dom/ContainerNode.cpp	2020-09-03 21:31:21 UTC (rev 266555)
+++ trunk/Source/WebCore/dom/ContainerNode.cpp	2020-09-03 21:35:17 UTC (rev 266556)
@@ -632,11 +632,6 @@
 // https://dom.spec.whatwg.org/#concept-node-replace-all
 void ContainerNode::replaceAllChildrenWithNewText(const String& text)
 {
-    if (text.isEmpty()) {
-        replaceAllChildren(nullptr);
-        return;
-    }
-
     auto node = document().createTextNode(text);
     if (!hasChildNodes()) {
         // appendChildWithoutPreInsertionValidityCheck() can only throw when node has a parent and we already asserted it doesn't.

Modified: trunk/Source/WebCore/dom/Node.cpp (266555 => 266556)


--- trunk/Source/WebCore/dom/Node.cpp	2020-09-03 21:31:21 UTC (rev 266555)
+++ trunk/Source/WebCore/dom/Node.cpp	2020-09-03 21:35:17 UTC (rev 266556)
@@ -1580,9 +1580,14 @@
     case PROCESSING_INSTRUCTION_NODE:
         return setNodeValue(text);
     case ELEMENT_NODE:
-    case DOCUMENT_FRAGMENT_NODE:
-        downcast<ContainerNode>(*this).replaceAllChildrenWithNewText(text);
+    case DOCUMENT_FRAGMENT_NODE: {
+        auto& container = downcast<ContainerNode>(*this);
+        if (text.isEmpty())
+            container.replaceAllChildren(nullptr);
+        else
+            container.replaceAllChildrenWithNewText(text);
         return { };
+    }
     case DOCUMENT_NODE:
     case DOCUMENT_TYPE_NODE:
         // Do nothing.

Modified: trunk/Source/WebCore/editing/ios/EditorIOS.mm (266555 => 266556)


--- trunk/Source/WebCore/editing/ios/EditorIOS.mm	2020-09-03 21:31:21 UTC (rev 266555)
+++ trunk/Source/WebCore/editing/ios/EditorIOS.mm	2020-09-03 21:35:17 UTC (rev 266556)
@@ -329,21 +329,49 @@
     clearUndoRedoOperations();
 
     // If the element is empty already and we're not adding text, we can early return and avoid clearing/setting
-    // a selection at [0, 0] and the expense involved in creating VisiblePositions.
+    // a selection at [0, 0] and the expense involved in creation VisiblePositions.
     if (!element.firstChild() && text.isEmpty())
         return;
 
-    // As a side effect this function sets a caret selection after the inserted content.
-    // What follows is more expensive if there is a selection, so clear it since it's going to change anyway.
+    // As a side effect this function sets a caret selection after the inserted content. Much of what
+    // follows is more expensive if there is a selection, so clear it since it's going to change anyway.
     m_document.selection().clear();
 
-    element.replaceAllChildrenWithNewText(text);
+    // clear out all current children of element
+    element.removeChildren();
 
-    VisiblePosition afterContents = createLegacyEditingPosition(&element, element.countChildNodes());
-    if (afterContents.isNull())
+    if (text.length()) {
+        // insert new text
+        // remove element from tree while doing it
+        // FIXME: The element we're inserting into is often the body element. It seems strange to be removing it
+        // (even if it is only temporary). ReplaceSelectionCommand doesn't bother doing this when it inserts
+        // content, why should we here?
+        RefPtr<Node> parent = element.parentNode();
+        RefPtr<Node> siblingAfter = element.nextSibling();
+        if (parent)
+            element.remove();
+
+        element.appendChild(createFragmentFromText(makeRangeSelectingNodeContents(element), text));
+
+        // restore element to document
+        if (parent) {
+            if (siblingAfter)
+                parent->insertBefore(element, siblingAfter.get());
+            else
+                parent->appendChild(element);
+        }
+    }
+
+    // set the selection to the end
+    VisiblePosition visiblePos(createLegacyEditingPosition(&element, element.countChildNodes()));
+    if (visiblePos.isNull())
         return;
-    m_document.selection().setSelection(afterContents);
 
+    VisibleSelection selection;
+    selection.setBase(visiblePos);
+    selection.setExtent(visiblePos);
+    m_document.selection().setSelection(selection);
+
     client()->respondToChangedContents();
 }
 

Modified: trunk/Source/WebCore/html/HTMLElement.cpp (266555 => 266556)


--- trunk/Source/WebCore/html/HTMLElement.cpp	2020-09-03 21:31:21 UTC (rev 266555)
+++ trunk/Source/WebCore/html/HTMLElement.cpp	2020-09-03 21:35:17 UTC (rev 266556)
@@ -539,7 +539,10 @@
     // FIXME: This doesn't take whitespace collapsing into account at all.
 
     if (!text.contains('\n') && !text.contains('\r')) {
-        replaceAllChildrenWithNewText(text);
+        if (text.isEmpty())
+            replaceAllChildren(nullptr);
+        else
+            replaceAllChildrenWithNewText(text);
         return { };
     }
 

Modified: trunk/Source/WebCore/page/DOMSelection.cpp (266555 => 266556)


--- trunk/Source/WebCore/page/DOMSelection.cpp	2020-09-03 21:31:21 UTC (rev 266555)
+++ trunk/Source/WebCore/page/DOMSelection.cpp	2020-09-03 21:35:17 UTC (rev 266556)
@@ -332,7 +332,7 @@
 
 void DOMSelection::deleteFromDocument()
 {
-    auto frame = makeRefPtr(this->frame());
+    auto* frame = this->frame();
     if (!frame)
         return;
 
@@ -340,8 +340,11 @@
     if (!selectedRange || selectedRange->start.container->containingShadowRoot())
         return;
 
+    Ref<Frame> protector(*frame);
     createLiveRange(*selectedRange)->deleteContents();
-    frame->selection().setSelectedRange(makeSimpleRange(selectedRange->start), Affinity::Upstream, FrameSelection::ShouldCloseTyping::No);
+    auto container = selectedRange->start.container.ptr();
+    auto offset = selectedRange->start.offset;
+    setBaseAndExtent(container, offset, container, offset);
 }
 
 bool DOMSelection::containsNode(Node& node, bool allowPartial) const
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to