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