Use Position instead of Range in createMarkupInternal
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 1 Oct 2018 05:30:02 +0000 (05:30 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 1 Oct 2018 05:30:02 +0000 (05:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=190107

Reviewed by Darin Adler.

Use two Position's indicating start and end instead of Range in createMarkupInternal and StylizedMarkupAccumulator
in order to support copy & paste across shadow boundaries in the bug 157443. This patch also removes the use of
Range in MarkupAccumulator since all uses of range is via StylizedMarkupAccumulator.

Also renamed createMarkupInternal to serializePreservingVisualAppearanceInternal to match the rename in r236612.

* dom/Position.cpp:
(WebCore::Position::firstNode const):  Added.
* dom/Position.h:
* editing/MarkupAccumulator.cpp:
(WebCore::MarkupAccumulator::MarkupAccumulator): No longer takes Range.
(WebCore::MarkupAccumulator::appendText): Removed the code to truncate string at the boundary points of the range.
* editing/MarkupAccumulator.h:
(WebCore::MarkupAccumulator): Made this class non-copyable.
* editing/markup.cpp:
(WebCore::StyledMarkupAccumulator::StyledMarkupAccumulator): Now takes and stores two positions.

(WebCore::StyledMarkupAccumulator::appendText): Use textContentRespectingRange in the case annotation is disabled
instead of calling to MarkupAccumulator::appendText, which no longer respects boundary offsets.

(WebCore::StyledMarkupAccumulator::renderedTextRespectingRange): Renamed from renderedText. Updated to respect
boundary offsets defined by m_start and m_end Positions instead of m_range Range.

(WebCore::StyledMarkupAccumulator::textContentRespectingRange): Renamed from stringValueForRange. Ditto.

(WebCore::StyledMarkupAccumulator::serializeNodes): Now computes startNode and pastEnd nodes from start and end
Positions. Note that the end position is always the next node in the tree order  for a character node
and computeNodeAfterPosition returns nullptr for a character data.

(WebCore::highestAncestorToWrapMarkup): Now takes two positions instead of a range.

(WebCore::serializePreservingVisualAppearanceInternal): Renamed from createMarkupInternal. Removed the obsolete
comments which were added for DOMRange in WebKitLegacy.

(WebCore::serializePreservingVisualAppearance):

(WebCore::sanitizedMarkupForFragmentInDocument): Create positions instead of a range to pass to
serializePreservingVisualAppearanceInternal.

(WebCore::serializeFragment):

* editing/markup.h:
* page/PageSerializer.cpp:
(WebCore::PageSerializer::SerializerMarkupAccumulator): Removed the unnecessary WebCore namespace qualifier.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@236649 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/dom/Position.cpp
Source/WebCore/dom/Position.h
Source/WebCore/editing/MarkupAccumulator.cpp
Source/WebCore/editing/MarkupAccumulator.h
Source/WebCore/editing/markup.cpp
Source/WebCore/page/PageSerializer.cpp

index ebcde60..55e35a9 100644 (file)
@@ -1,3 +1,55 @@
+2018-09-30  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Use Position instead of Range in createMarkupInternal
+        https://bugs.webkit.org/show_bug.cgi?id=190107
+
+        Reviewed by Darin Adler.
+
+        Use two Position's indicating start and end instead of Range in createMarkupInternal and StylizedMarkupAccumulator
+        in order to support copy & paste across shadow boundaries in the bug 157443. This patch also removes the use of
+        Range in MarkupAccumulator since all uses of range is via StylizedMarkupAccumulator.
+
+        Also renamed createMarkupInternal to serializePreservingVisualAppearanceInternal to match the rename in r236612.
+
+        * dom/Position.cpp:
+        (WebCore::Position::firstNode const):  Added.
+        * dom/Position.h:
+        * editing/MarkupAccumulator.cpp:
+        (WebCore::MarkupAccumulator::MarkupAccumulator): No longer takes Range.
+        (WebCore::MarkupAccumulator::appendText): Removed the code to truncate string at the boundary points of the range.
+        * editing/MarkupAccumulator.h:
+        (WebCore::MarkupAccumulator): Made this class non-copyable.
+        * editing/markup.cpp:
+        (WebCore::StyledMarkupAccumulator::StyledMarkupAccumulator): Now takes and stores two positions.
+
+        (WebCore::StyledMarkupAccumulator::appendText): Use textContentRespectingRange in the case annotation is disabled
+        instead of calling to MarkupAccumulator::appendText, which no longer respects boundary offsets.
+
+        (WebCore::StyledMarkupAccumulator::renderedTextRespectingRange): Renamed from renderedText. Updated to respect
+        boundary offsets defined by m_start and m_end Positions instead of m_range Range.
+
+        (WebCore::StyledMarkupAccumulator::textContentRespectingRange): Renamed from stringValueForRange. Ditto.
+
+        (WebCore::StyledMarkupAccumulator::serializeNodes): Now computes startNode and pastEnd nodes from start and end
+        Positions. Note that the end position is always the next node in the tree order  for a character node
+        and computeNodeAfterPosition returns nullptr for a character data.
+
+        (WebCore::highestAncestorToWrapMarkup): Now takes two positions instead of a range.
+
+        (WebCore::serializePreservingVisualAppearanceInternal): Renamed from createMarkupInternal. Removed the obsolete
+        comments which were added for DOMRange in WebKitLegacy.
+
+        (WebCore::serializePreservingVisualAppearance):
+
+        (WebCore::sanitizedMarkupForFragmentInDocument): Create positions instead of a range to pass to
+        serializePreservingVisualAppearanceInternal.
+
+        (WebCore::serializeFragment):
+
+        * editing/markup.h:
+        * page/PageSerializer.cpp:
+        (WebCore::PageSerializer::SerializerMarkupAccumulator): Removed the unnecessary WebCore namespace qualifier.
+
 2018-09-30  Walker Henderson  <wjahenderson@gmail.com>
 
         AudioNode.connect should return passed destination node
index 02f1c71..d167a7e 100644 (file)
@@ -38,6 +38,7 @@
 #include "InlineIterator.h"
 #include "InlineTextBox.h"
 #include "Logging.h"
+#include "NodeTraversal.h"
 #include "PositionIterator.h"
 #include "RenderBlock.h"
 #include "RenderFlexibleBox.h"
@@ -247,6 +248,20 @@ Position Position::parentAnchoredEquivalent() const
     return { containerNode(), computeOffsetInContainerNode(), PositionIsOffsetInAnchor };
 }
 
+RefPtr<Node> Position::firstNode() const
+{
+    auto container = makeRefPtr(containerNode());
+    if (!container)
+        return nullptr;
+    if (is<CharacterData>(*container))
+        return container;
+    if (auto* node = computeNodeAfterPosition())
+        return node;
+    if (!computeOffsetInContainerNode())
+        return container;
+    return NodeTraversal::nextSkippingChildren(*container);
+}
+
 Node* Position::computeNodeBeforePosition() const
 {
     if (!m_anchorNode)
index b5f7390..d94e354 100644 (file)
@@ -103,6 +103,8 @@ public:
         return offsetForPositionAfterAnchor();
     }
 
+    RefPtr<Node> firstNode() const;
+
     // These are convenience methods which are smart about whether the position is neighbor anchored or parent anchored
     Node* computeNodeBeforePosition() const;
     Node* computeNodeAfterPosition() const;
index 09942b7..353dba6 100644 (file)
@@ -116,12 +116,10 @@ void MarkupAccumulator::appendCharactersReplacingEntities(StringBuilder& result,
         appendCharactersReplacingEntitiesInternal<UChar>(result, source, offset, length, entityMask);
 }
 
-MarkupAccumulator::MarkupAccumulator(Vector<Node*>* nodes, ResolveURLs resolveURLs, const Range* range, SerializationSyntax serializationSyntax)
+MarkupAccumulator::MarkupAccumulator(Vector<Node*>* nodes, ResolveURLs resolveURLs, SerializationSyntax serializationSyntax)
     : m_nodes(nodes)
-    , m_range(range)
     , m_resolveURLs(resolveURLs)
     , m_serializationSyntax(serializationSyntax)
-    , m_prefixLevel(0)
 {
 }
 
@@ -341,19 +339,7 @@ EntityMask MarkupAccumulator::entityMaskForText(const Text& text) const
 void MarkupAccumulator::appendText(StringBuilder& result, const Text& text)
 {
     const String& textData = text.data();
-    unsigned start = 0;
-    unsigned length = textData.length();
-
-    if (m_range) {
-        if (&text == &m_range->endContainer())
-            length = m_range->endOffset();
-        if (&text == &m_range->startContainer()) {
-            start = m_range->startOffset();
-            length -= start;
-        }
-    }
-
-    appendCharactersReplacingEntities(result, textData, start, length, entityMaskForText(text));
+    appendCharactersReplacingEntities(result, textData, 0, textData.length(), entityMaskForText(text));
 }
 
 static void appendComment(StringBuilder& result, const String& comment)
index bee8681..3311c0c 100644 (file)
@@ -56,10 +56,10 @@ enum EntityMask {
     EntityMaskInHTMLAttributeValue = EntityAmp | EntityQuot | EntityNbsp,
 };
 
-// FIXME: Noncopyable?
 class MarkupAccumulator {
+    WTF_MAKE_NONCOPYABLE(MarkupAccumulator);
 public:
-    MarkupAccumulator(Vector<Node*>*, ResolveURLs, const Range* = nullptr, SerializationSyntax = SerializationSyntax::HTML);
+    MarkupAccumulator(Vector<Node*>*, ResolveURLs, SerializationSyntax = SerializationSyntax::HTML);
     virtual ~MarkupAccumulator();
 
     String serializeNodes(Node& targetNode, SerializedNodes, Vector<QualifiedName>* tagNamesToSkip = nullptr);
@@ -109,7 +109,6 @@ protected:
     EntityMask entityMaskForText(const Text&) const;
 
     Vector<Node*>* const m_nodes;
-    const Range* const m_range;
 
 private:
     String resolveURLIfNeeded(const Element&, const String&) const;
@@ -121,7 +120,7 @@ private:
     StringBuilder m_markup;
     const ResolveURLs m_resolveURLs;
     SerializationSyntax m_serializationSyntax;
-    unsigned m_prefixLevel;
+    unsigned m_prefixLevel { 0 };
 };
 
 } // namespace WebCore
index 912f583..f94fb31 100644 (file)
@@ -219,9 +219,10 @@ class StyledMarkupAccumulator final : public MarkupAccumulator {
 public:
     enum RangeFullySelectsNode { DoesFullySelectNode, DoesNotFullySelectNode };
 
-    StyledMarkupAccumulator(Vector<Node*>* nodes, ResolveURLs, AnnotateForInterchange, MSOListMode, const Range*, bool needsPositionStyleConversion, Node* highestNodeToBeSerialized = nullptr);
+    StyledMarkupAccumulator(const Position& start, const Position& end, Vector<Node*>* nodes,
+        ResolveURLs, AnnotateForInterchange, MSOListMode, bool needsPositionStyleConversion, Node* highestNodeToBeSerialized = nullptr);
 
-    Node* serializeNodes(Node* startNode, Node* pastEnd);
+    Node* serializeNodes(const Position& start, const Position& end);
     void wrapWithNode(Node&, bool convertBlocksToInlines = false, RangeFullySelectsNode = DoesFullySelectNode);
     void wrapWithStyleNode(StyleProperties*, Document&, bool isBlock = false);
     String takeResults();
@@ -235,8 +236,8 @@ private:
     void appendStyleNodeOpenTag(StringBuilder&, StyleProperties*, Document&, bool isBlock = false);
     const String& styleNodeCloseTag(bool isBlock = false);
 
-    String renderedText(const Node&, const Range*);
-    String stringValueForRange(const Node&, const Range*);
+    String renderedTextRespectingRange(const Text&);
+    String textContentRespectingRange(const Text&);
 
     bool shouldPreserveMSOListStyleForElement(const Element&);
 
@@ -264,24 +265,27 @@ private:
         return m_highestNodeToBeSerialized && m_highestNodeToBeSerialized->parentNode() == node.parentNode() && m_wrappingStyle && m_wrappingStyle->style();
     }
 
+    Position m_start;
+    Position m_end;
     Vector<String> m_reversedPrecedingMarkup;
     const AnnotateForInterchange m_annotate;
-    Node* m_highestNodeToBeSerialized;
+    RefPtr<Node> m_highestNodeToBeSerialized;
     RefPtr<EditingStyle> m_wrappingStyle;
-    bool m_needRelativeStyleWrapper;
     bool m_needsPositionStyleConversion;
-    bool m_needClearingDiv;
+    bool m_needRelativeStyleWrapper { false };
+    bool m_needClearingDiv { false };
     bool m_shouldPreserveMSOList;
     bool m_inMSOList { false };
 };
 
-inline StyledMarkupAccumulator::StyledMarkupAccumulator(Vector<Node*>* nodes, ResolveURLs urlsToResolve, AnnotateForInterchange annotate, MSOListMode msoListMode, const Range* range, bool needsPositionStyleConversion, Node* highestNodeToBeSerialized)
-    : MarkupAccumulator(nodes, urlsToResolve, range)
+inline StyledMarkupAccumulator::StyledMarkupAccumulator(const Position& start, const Position& end, Vector<Node*>* nodes,
+    ResolveURLs urlsToResolve, AnnotateForInterchange annotate, MSOListMode msoListMode, bool needsPositionStyleConversion, Node* highestNodeToBeSerialized)
+    : MarkupAccumulator(nodes, urlsToResolve)
+    , m_start(start)
+    , m_end(end)
     , m_annotate(annotate)
     , m_highestNodeToBeSerialized(highestNodeToBeSerialized)
-    , m_needRelativeStyleWrapper(false)
     , m_needsPositionStyleConversion(needsPositionStyleConversion)
-    , m_needClearingDiv(false)
     , m_shouldPreserveMSOList(msoListMode == MSOListMode::Preserve)
 {
 }
@@ -355,11 +359,12 @@ void StyledMarkupAccumulator::appendText(StringBuilder& out, const Text& text)
         appendStyleNodeOpenTag(out, wrappingStyle->style(), text.document());
     }
 
-    if (!shouldAnnotate() || parentIsTextarea)
-        MarkupAccumulator::appendText(out, text);
-    else {
+    if (!shouldAnnotate() || parentIsTextarea) {
+        auto content = textContentRespectingRange(text);
+        appendCharactersReplacingEntities(out, content, 0, content.length(), entityMaskForText(text));
+    } else {
         const bool useRenderedText = !enclosingElementWithTag(firstPositionInNode(const_cast<Text*>(&text)), selectTag);
-        String content = useRenderedText ? renderedText(text, m_range) : stringValueForRange(text, m_range);
+        String content = useRenderedText ? renderedTextRespectingRange(text) : textContentRespectingRange(text);
         StringBuilder buffer;
         appendCharactersReplacingEntities(buffer, content, 0, content.length(), EntityMaskInPCDATA);
         out.append(convertHTMLTextToInterchangeFormat(buffer.toString(), &text));
@@ -369,39 +374,35 @@ void StyledMarkupAccumulator::appendText(StringBuilder& out, const Text& text)
         out.append(styleNodeCloseTag());
 }
     
-String StyledMarkupAccumulator::renderedText(const Node& node, const Range* range)
+String StyledMarkupAccumulator::renderedTextRespectingRange(const Text& text)
 {
-    if (!is<Text>(node))
-        return String();
-
-    const Text& textNode = downcast<Text>(node);
-    unsigned startOffset = 0;
-    unsigned endOffset = textNode.length();
-
     TextIteratorBehavior behavior = TextIteratorDefaultBehavior;
-    if (range && &node == &range->startContainer())
-        startOffset = range->startOffset();
-    if (range && &node == &range->endContainer())
-        endOffset = range->endOffset();
-    else if (range)
-        behavior = TextIteratorBehavesAsIfNodesFollowing;
-
-    Position start = createLegacyEditingPosition(const_cast<Node*>(&node), startOffset);
-    Position end = createLegacyEditingPosition(const_cast<Node*>(&node), endOffset);
-    return plainText(Range::create(node.document(), start, end).ptr(), behavior);
+    Position start = &text == m_start.containerNode() ? m_start : firstPositionInNode(const_cast<Text*>(&text));
+    Position end;
+    if (&text == m_end.containerNode())
+        end = m_end;
+    else {
+        end = lastPositionInNode(const_cast<Text*>(&text));
+        if (!m_end.isNull())
+            behavior = TextIteratorBehavesAsIfNodesFollowing;
+    }
+
+    return plainText(Range::create(text.document(), start, end).ptr(), behavior);
 }
 
-String StyledMarkupAccumulator::stringValueForRange(const Node& node, const Range* range)
+String StyledMarkupAccumulator::textContentRespectingRange(const Text& text)
 {
-    if (!range)
-        return node.nodeValue();
-
-    String nodeValue = node.nodeValue();
-    if (&node == &range->endContainer())
-        nodeValue.truncate(range->endOffset());
-    if (&node == &range->startContainer())
-        nodeValue.remove(0, range->startOffset());
-    return nodeValue;
+    if (m_start.isNull() && m_end.isNull())
+        return text.data();
+
+    unsigned start = 0;
+    unsigned end = std::numeric_limits<unsigned>::max();
+    if (&text == m_start.containerNode())
+        start = m_start.offsetInContainerNode();
+    if (&text == m_end.containerNode())
+        end = m_end.offsetInContainerNode();
+    ASSERT(start < end);
+    return text.data().substring(start, end - start);
 }
 
 void StyledMarkupAccumulator::appendCustomAttributes(StringBuilder& out, const Element& element, Namespaces* namespaces)
@@ -502,17 +503,23 @@ void StyledMarkupAccumulator::appendElement(StringBuilder& out, const Element& e
     appendCloseTag(out, element);
 }
 
-Node* StyledMarkupAccumulator::serializeNodes(Node* startNode, Node* pastEnd)
+Node* StyledMarkupAccumulator::serializeNodes(const Position& start, const Position& end)
 {
+    ASSERT(comparePositions(start, end) <= 0);
+    auto startNode = start.firstNode();
+    Node* pastEnd = end.computeNodeAfterPosition();
+    if (!pastEnd)
+        pastEnd = NodeTraversal::nextSkippingChildren(*end.containerNode());
+
     if (!m_highestNodeToBeSerialized) {
-        Node* lastClosed = traverseNodesForSerialization(startNode, pastEnd, NodeTraversalMode::DoNotEmitString);
+        Node* lastClosed = traverseNodesForSerialization(startNode.get(), pastEnd, NodeTraversalMode::DoNotEmitString);
         m_highestNodeToBeSerialized = lastClosed;
     }
 
     if (m_highestNodeToBeSerialized && m_highestNodeToBeSerialized->parentNode())
         m_wrappingStyle = EditingStyle::wrappingStyleForSerialization(*m_highestNodeToBeSerialized->parentNode(), shouldAnnotate());
 
-    return traverseNodesForSerialization(startNode, pastEnd, NodeTraversalMode::EmitString);
+    return traverseNodesForSerialization(startNode.get(), pastEnd, NodeTraversalMode::EmitString);
 }
 
 Node* StyledMarkupAccumulator::traverseNodesForSerialization(Node* startNode, Node* pastEnd, NodeTraversalMode traversalMode)
@@ -704,18 +711,16 @@ static bool isElementPresentational(const Node* node)
         || node->hasTagName(iTag) || node->hasTagName(emTag) || node->hasTagName(bTag) || node->hasTagName(strongTag);
 }
 
-static Node* highestAncestorToWrapMarkup(const Range* range, AnnotateForInterchange annotate)
+static Node* highestAncestorToWrapMarkup(const Position& start, const Position& end, Node& commonAncestor, AnnotateForInterchange annotate)
 {
-    auto* commonAncestor = range->commonAncestorContainer();
-    ASSERT(commonAncestor);
     Node* specialCommonAncestor = nullptr;
     if (annotate == AnnotateForInterchange::Yes) {
         // Include ancestors that aren't completely inside the range but are required to retain 
         // the structure and appearance of the copied markup.
-        specialCommonAncestor = ancestorToRetainStructureAndAppearance(commonAncestor);
+        specialCommonAncestor = ancestorToRetainStructureAndAppearance(&commonAncestor);
 
-        if (auto* parentListNode = enclosingNodeOfType(firstPositionInOrBeforeNode(range->firstNode()), isListItem)) {
-            if (!editingIgnoresContent(*parentListNode) && WebCore::areRangesEqual(VisibleSelection::selectionFromContentsOfNode(parentListNode).toNormalizedRange().get(), range)) {
+        if (auto* parentListNode = enclosingNodeOfType(start, isListItem)) {
+            if (!editingIgnoresContent(*parentListNode) && VisibleSelection::selectionFromContentsOfNode(parentListNode) == VisibleSelection(start, end)) {
                 specialCommonAncestor = parentListNode->parentNode();
                 while (specialCommonAncestor && !isListHTMLElement(specialCommonAncestor))
                     specialCommonAncestor = specialCommonAncestor->parentNode();
@@ -723,11 +728,11 @@ static Node* highestAncestorToWrapMarkup(const Range* range, AnnotateForIntercha
         }
 
         // Retain the Mail quote level by including all ancestor mail block quotes.
-        if (Node* highestMailBlockquote = highestEnclosingNodeOfType(firstPositionInOrBeforeNode(range->firstNode()), isMailBlockquote, CanCrossEditingBoundary))
+        if (Node* highestMailBlockquote = highestEnclosingNodeOfType(start, isMailBlockquote, CanCrossEditingBoundary))
             specialCommonAncestor = highestMailBlockquote;
     }
 
-    auto* checkAncestor = specialCommonAncestor ? specialCommonAncestor : commonAncestor;
+    auto* checkAncestor = specialCommonAncestor ? specialCommonAncestor : &commonAncestor;
     if (checkAncestor->renderer() && checkAncestor->renderer()->containingBlock()) {
         Node* newSpecialCommonAncestor = highestEnclosingNodeOfType(firstPositionInNode(checkAncestor), &isElementPresentational, CanCrossEditingBoundary, checkAncestor->renderer()->containingBlock()->element());
         if (newSpecialCommonAncestor)
@@ -738,61 +743,59 @@ static Node* highestAncestorToWrapMarkup(const Range* range, AnnotateForIntercha
     // If two or more tabs are selected, commonAncestor will be the tab span.
     // In either case, if there is a specialCommonAncestor already, it will necessarily be above 
     // any tab span that needs to be included.
-    if (!specialCommonAncestor && isTabSpanTextNode(commonAncestor))
-        specialCommonAncestor = commonAncestor->parentNode();
-    if (!specialCommonAncestor && isTabSpanNode(commonAncestor))
-        specialCommonAncestor = commonAncestor;
+    if (!specialCommonAncestor && isTabSpanTextNode(&commonAncestor))
+        specialCommonAncestor = commonAncestor.parentNode();
+    if (!specialCommonAncestor && isTabSpanNode(&commonAncestor))
+        specialCommonAncestor = &commonAncestor;
 
-    if (auto* enclosingAnchor = enclosingElementWithTag(firstPositionInNode(specialCommonAncestor ? specialCommonAncestor : commonAncestor), aTag))
+    if (auto* enclosingAnchor = enclosingElementWithTag(firstPositionInNode(specialCommonAncestor ? specialCommonAncestor : &commonAncestor), aTag))
         specialCommonAncestor = enclosingAnchor;
 
     return specialCommonAncestor;
 }
 
-// FIXME: Shouldn't we omit style info when annotate == AnnotateForInterchange::No?
-// FIXME: At least, annotation and style info should probably not be included in range.markupString()
-static String createMarkupInternal(Document& document, const Range& range, Vector<Node*>* nodes,
+static String serializePreservingVisualAppearanceInternal(const Position& start, const Position& end, Vector<Node*>* nodes,
     AnnotateForInterchange annotate, ConvertBlocksToInlines convertBlocksToInlines, ResolveURLs urlsToResolve, MSOListMode msoListMode)
 {
     static NeverDestroyed<const String> interchangeNewlineString(MAKE_STATIC_STRING_IMPL("<br class=\"" AppleInterchangeNewline "\">"));
 
-    bool collapsed = range.collapsed();
-    if (collapsed)
+    if (!comparePositions(start, end))
         return emptyString();
-    RefPtr<Node> commonAncestor = range.commonAncestorContainer();
+
+    RefPtr<Node> commonAncestor = Range::commonAncestorContainer(start.containerNode(), end.containerNode());
     if (!commonAncestor)
         return emptyString();
 
+    auto& document = *start.document();
     document.updateLayoutIgnorePendingStylesheets();
 
-    auto* body = enclosingElementWithTag(firstPositionInNode(commonAncestor.get()), bodyTag);
-    Element* fullySelectedRoot = nullptr;
+    VisiblePosition visibleStart { start };
+    VisiblePosition visibleEnd { end };
+
+    auto body = makeRefPtr(enclosingElementWithTag(firstPositionInNode(commonAncestor.get()), bodyTag));
+    RefPtr<Element> fullySelectedRoot;
     // FIXME: Do this for all fully selected blocks, not just the body.
-    if (body && VisiblePosition(firstPositionInNode(body)) == VisiblePosition(range.startPosition())
-        && VisiblePosition(lastPositionInNode(body)) == VisiblePosition(range.endPosition()))
+    if (body && VisiblePosition(firstPositionInNode(body.get())) == visibleStart && VisiblePosition(lastPositionInNode(body.get())) == visibleEnd)
         fullySelectedRoot = body;
-    Node* specialCommonAncestor = highestAncestorToWrapMarkup(&range, annotate);
+    bool needsPositionStyleConversion = body && fullySelectedRoot == body && document.settings().shouldConvertPositionStyleOnCopy();
+
+    Node* specialCommonAncestor = highestAncestorToWrapMarkup(start, end, *commonAncestor, annotate);
 
-    bool needsPositionStyleConversion = body && fullySelectedRoot == body
-        && document.settings().shouldConvertPositionStyleOnCopy();
-    StyledMarkupAccumulator accumulator(nodes, urlsToResolve, annotate, msoListMode, &range, needsPositionStyleConversion, specialCommonAncestor);
-    Node* pastEnd = range.pastLastNode();
+    StyledMarkupAccumulator accumulator(start, end, nodes, urlsToResolve, annotate, msoListMode, needsPositionStyleConversion, specialCommonAncestor);
 
-    Node* startNode = range.firstNode();
-    VisiblePosition visibleStart(range.startPosition(), VP_DEFAULT_AFFINITY);
-    VisiblePosition visibleEnd(range.endPosition(), VP_DEFAULT_AFFINITY);
+    Position startAdjustedForInterchangeNewline = start;
     if (annotate == AnnotateForInterchange::Yes && needInterchangeNewlineAfter(visibleStart)) {
         if (visibleStart == visibleEnd.previous())
             return interchangeNewlineString;
 
         accumulator.appendString(interchangeNewlineString);
-        startNode = visibleStart.next().deepEquivalent().deprecatedNode();
+        startAdjustedForInterchangeNewline = visibleStart.next().deepEquivalent();
 
-        if (pastEnd && Range::compareBoundaryPoints(startNode, 0, pastEnd, 0).releaseReturnValue() >= 0)
+        if (comparePositions(startAdjustedForInterchangeNewline, end) >= 0)
             return interchangeNewlineString;
     }
 
-    Node* lastClosed = accumulator.serializeNodes(startNode, pastEnd);
+    Node* lastClosed = accumulator.serializeNodes(startAdjustedForInterchangeNewline, end);
 
     if (specialCommonAncestor && lastClosed) {
         // Also include all of the ancestors of lastClosed up to this special ancestor.
@@ -846,7 +849,7 @@ static String createMarkupInternal(Document& document, const Range& range, Vecto
 
 String serializePreservingVisualAppearance(const Range& range, Vector<Node*>* nodes, AnnotateForInterchange annotate, ConvertBlocksToInlines convertBlocksToInlines, ResolveURLs urlsToReslve)
 {
-    return createMarkupInternal(range.ownerDocument(), range, nodes, annotate, convertBlocksToInlines, urlsToReslve, MSOListMode::DoNotPreserve);
+    return serializePreservingVisualAppearanceInternal(range.startPosition(), range.endPosition(), nodes, annotate, convertBlocksToInlines, urlsToReslve, MSOListMode::DoNotPreserve);
 }
 
 static bool shouldPreserveMSOLists(const String& markup)
@@ -870,9 +873,8 @@ String sanitizedMarkupForFragmentInDocument(Ref<DocumentFragment>&& fragment, Do
     ASSERT(bodyElement);
     bodyElement->appendChild(fragment.get());
 
-    auto range = Range::create(document);
-    range->selectNodeContents(*bodyElement);
-    auto result = createMarkupInternal(document, range.get(), nullptr, AnnotateForInterchange::Yes, ConvertBlocksToInlines::No, ResolveURLs::YesExcludingLocalFileURLsForPrivacy, msoListMode);
+    auto result = serializePreservingVisualAppearanceInternal(firstPositionInNode(bodyElement.get()), lastPositionInNode(bodyElement.get()), nullptr,
+        AnnotateForInterchange::Yes, ConvertBlocksToInlines::No, ResolveURLs::YesExcludingLocalFileURLsForPrivacy, msoListMode);
 
     if (msoListMode == MSOListMode::Preserve) {
         StringBuilder builder;
@@ -950,7 +952,7 @@ Ref<DocumentFragment> createFragmentFromMarkup(Document& document, const String&
 
 String serializeFragment(const Node& node, SerializedNodes root, Vector<Node*>* nodes, ResolveURLs urlsToResolve, Vector<QualifiedName>* tagNamesToSkip, SerializationSyntax serializationSyntax)
 {
-    MarkupAccumulator accumulator(nodes, urlsToResolve, nullptr, serializationSyntax);
+    MarkupAccumulator accumulator(nodes, urlsToResolve, serializationSyntax);
     return accumulator.serializeNodes(const_cast<Node&>(node), root, tagNamesToSkip);
 }
 
index 5f0fe76..384750d 100644 (file)
@@ -94,7 +94,7 @@ static const QualifiedName& frameOwnerURLAttributeName(const HTMLFrameOwnerEleme
     return is<HTMLObjectElement>(frameOwner) ? HTMLNames::dataAttr : HTMLNames::srcAttr;
 }
 
-class PageSerializer::SerializerMarkupAccumulator final : public WebCore::MarkupAccumulator {
+class PageSerializer::SerializerMarkupAccumulator final : public MarkupAccumulator {
 public:
     SerializerMarkupAccumulator(PageSerializer&, Document&, Vector<Node*>*);