Title: [266498] trunk/Source/WebCore
- Revision
- 266498
- Author
- [email protected]
- Date
- 2020-09-02 18:32:41 -0700 (Wed, 02 Sep 2020)
Log Message
Simplify some editing code
https://bugs.webkit.org/show_bug.cgi?id=216097
Reviewed by Sam Weinig.
* dom/ContainerNode.cpp:
(WebCore::ContainerNode::replaceAllChildrenWithNewText):
If string is empty, don't add a text node. Turns out all callers wanted
this behavior.
* dom/Node.cpp:
(WebCore::Node::setTextContent): Simplify by relying on new behavior
of replaceAllChildrenWithNewText when passed an empty string.
* editing/ios/EditorIOS.mm:
(WebCore::Editor::setTextAsChildOfElement): Use
replaceAllChildrenWithNewText instead of the unnecessarily complicated
code that was here. Also simplify the VisibeSelection manipulation at
the end of the function.
* html/HTMLElement.cpp:
(WebCore::HTMLElement::setInnerText): Simplify by relying on new
behavior of replaceAllChildrenWithNewText when passed an empty string.
* page/DOMSelection.cpp:
(WebCore::DOMSelection::deleteFromDocument): Simplify by using the
selection function that takes a SimpleRange rather than the one that
takes two containers and offsets.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (266497 => 266498)
--- trunk/Source/WebCore/ChangeLog 2020-09-03 01:04:44 UTC (rev 266497)
+++ trunk/Source/WebCore/ChangeLog 2020-09-03 01:32:41 UTC (rev 266498)
@@ -1,3 +1,34 @@
+2020-09-02 Darin Adler <[email protected]>
+
+ Simplify some editing code
+ https://bugs.webkit.org/show_bug.cgi?id=216097
+
+ Reviewed by Sam Weinig.
+
+ * dom/ContainerNode.cpp:
+ (WebCore::ContainerNode::replaceAllChildrenWithNewText):
+ If string is empty, don't add a text node. Turns out all callers wanted
+ this behavior.
+
+ * dom/Node.cpp:
+ (WebCore::Node::setTextContent): Simplify by relying on new behavior
+ of replaceAllChildrenWithNewText when passed an empty string.
+
+ * editing/ios/EditorIOS.mm:
+ (WebCore::Editor::setTextAsChildOfElement): Use
+ replaceAllChildrenWithNewText instead of the unnecessarily complicated
+ code that was here. Also simplify the VisibeSelection manipulation at
+ the end of the function.
+
+ * html/HTMLElement.cpp:
+ (WebCore::HTMLElement::setInnerText): Simplify by relying on new
+ behavior of replaceAllChildrenWithNewText when passed an empty string.
+
+ * page/DOMSelection.cpp:
+ (WebCore::DOMSelection::deleteFromDocument): Simplify by using the
+ selection function that takes a SimpleRange rather than the one that
+ takes two containers and offsets.
+
2020-09-02 Chris Dumez <[email protected]>
Don't modify the response when creating a ConvolverNode
Modified: trunk/Source/WebCore/dom/ContainerNode.cpp (266497 => 266498)
--- trunk/Source/WebCore/dom/ContainerNode.cpp 2020-09-03 01:04:44 UTC (rev 266497)
+++ trunk/Source/WebCore/dom/ContainerNode.cpp 2020-09-03 01:32:41 UTC (rev 266498)
@@ -632,6 +632,11 @@
// 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 (266497 => 266498)
--- trunk/Source/WebCore/dom/Node.cpp 2020-09-03 01:04:44 UTC (rev 266497)
+++ trunk/Source/WebCore/dom/Node.cpp 2020-09-03 01:32:41 UTC (rev 266498)
@@ -1580,14 +1580,9 @@
case PROCESSING_INSTRUCTION_NODE:
return setNodeValue(text);
case ELEMENT_NODE:
- case DOCUMENT_FRAGMENT_NODE: {
- auto& container = downcast<ContainerNode>(*this);
- if (text.isEmpty())
- container.replaceAllChildren(nullptr);
- else
- container.replaceAllChildrenWithNewText(text);
+ case DOCUMENT_FRAGMENT_NODE:
+ downcast<ContainerNode>(*this).replaceAllChildrenWithNewText(text);
return { };
- }
case DOCUMENT_NODE:
case DOCUMENT_TYPE_NODE:
// Do nothing.
Modified: trunk/Source/WebCore/editing/ios/EditorIOS.mm (266497 => 266498)
--- trunk/Source/WebCore/editing/ios/EditorIOS.mm 2020-09-03 01:04:44 UTC (rev 266497)
+++ trunk/Source/WebCore/editing/ios/EditorIOS.mm 2020-09-03 01:32:41 UTC (rev 266498)
@@ -329,49 +329,21 @@
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 creation VisiblePositions.
+ // a selection at [0, 0] and the expense involved in creating VisiblePositions.
if (!element.firstChild() && text.isEmpty())
return;
- // 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.
+ // 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.
m_document.selection().clear();
- // clear out all current children of element
- element.removeChildren();
+ element.replaceAllChildrenWithNewText(text);
- 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())
+ VisiblePosition afterContents = createLegacyEditingPosition(&element, element.countChildNodes());
+ if (afterContents.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 (266497 => 266498)
--- trunk/Source/WebCore/html/HTMLElement.cpp 2020-09-03 01:04:44 UTC (rev 266497)
+++ trunk/Source/WebCore/html/HTMLElement.cpp 2020-09-03 01:32:41 UTC (rev 266498)
@@ -539,10 +539,7 @@
// FIXME: This doesn't take whitespace collapsing into account at all.
if (!text.contains('\n') && !text.contains('\r')) {
- if (text.isEmpty())
- replaceAllChildren(nullptr);
- else
- replaceAllChildrenWithNewText(text);
+ replaceAllChildrenWithNewText(text);
return { };
}
Modified: trunk/Source/WebCore/page/DOMSelection.cpp (266497 => 266498)
--- trunk/Source/WebCore/page/DOMSelection.cpp 2020-09-03 01:04:44 UTC (rev 266497)
+++ trunk/Source/WebCore/page/DOMSelection.cpp 2020-09-03 01:32:41 UTC (rev 266498)
@@ -332,7 +332,7 @@
void DOMSelection::deleteFromDocument()
{
- auto* frame = this->frame();
+ auto frame = makeRefPtr(this->frame());
if (!frame)
return;
@@ -340,11 +340,8 @@
if (!selectedRange || selectedRange->start.container->containingShadowRoot())
return;
- Ref<Frame> protector(*frame);
createLiveRange(*selectedRange)->deleteContents();
- auto container = selectedRange->start.container.ptr();
- auto offset = selectedRange->start.offset;
- setBaseAndExtent(container, offset, container, offset);
+ frame->selection().setSelectedRange(makeSimpleRange(selectedRange->start), Affinity::Upstream, FrameSelection::ShouldCloseTyping::No);
}
bool DOMSelection::containsNode(Node& node, bool allowPartial) const
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
