Attachment 357099 Details for Bug 192613 – Cleanup

[patch] Cleanup

bug-192613-20181211214250.patch (text/plain), 9.06 KB, created by Ryosuke Niwa on 2018-12-11 21:42:50 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Ryosuke Niwa

Created: 2018-12-11 21:42:50 PST

Size: 9.06 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 239097)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,30 @@
>+2018-12-11  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        Make HTMLConverter take two Positions in preparation to make it work with shadow DOM
>+        https://bugs.webkit.org/show_bug.cgi?id=192613
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This patch makes HTMLConverter store two Position's instead of a Range so that HTMLConverter can work with
>+        a selection which spans across shadow boundaries in the future.
>+
>+        No new tests since there should be no observable behavioral change.
>+
>+        * editing/cocoa/EditorCocoa.mm:
>+        (WebCore::Editor::writeSelectionToPasteboard): Uses the newly introduced writeSelectionToPasteboard.
>+        (WebCore::Editor::writeSelection): Ditto.
>+        * editing/cocoa/HTMLConverter.h:
>+        * editing/cocoa/HTMLConverter.mm:
>+        (HTMLConverter::HTMLConverter): Now takes two Position's.
>+        (HTMLConverter::convert): Updated to work with Position's.
>+        (HTMLConverter::_processText): Ditto.
>+        (HTMLConverter::_traverseNode): Ditto.
>+        (HTMLConverter::_traverseFooterNode): Ditto.
>+        (HTMLConverterCaches::cacheAncestorsOfStartToBeConverted): Ditto.
>+        (WebCore::attributedStringFromRange): Ditto.
>+        (WebCore::attributedStringFromSelection): Added. For now, we first create a Range via toNormalizedRange
>+        in order to preserve the exact behavior.
>+
> 2018-12-10  Ryosuke Niwa  <rniwa@webkit.org>
> 
>         connectedCallback is invoked during the removal of the element inside another element's connectedCallback
>Index: Source/WebCore/editing/cocoa/EditorCocoa.mm
>===================================================================
>--- Source/WebCore/editing/cocoa/EditorCocoa.mm	(revision 239063)
>+++ Source/WebCore/editing/cocoa/EditorCocoa.mm	(working copy)
>@@ -103,7 +103,7 @@ void Editor::getPasteboardTypesAndDataFo
> 
> void Editor::writeSelectionToPasteboard(Pasteboard& pasteboard)
> {
>-    NSAttributedString *attributedString = attributedStringFromRange(*selectedRange());
>+    NSAttributedString *attributedString = attributedStringFromSelection(m_frame.selection().selection());
> 
>     PasteboardWebContent content;
>     content.contentOrigin = m_frame.document()->originIdentifierForPasteboard();
>@@ -121,7 +121,7 @@ void Editor::writeSelectionToPasteboard(
> 
> void Editor::writeSelection(PasteboardWriterData& pasteboardWriterData)
> {
>-    NSAttributedString *attributedString = attributedStringFromRange(*selectedRange());
>+    NSAttributedString *attributedString = attributedStringFromSelection(m_frame.selection().selection());
> 
>     PasteboardWriterData::WebContent webContent;
>     webContent.contentOrigin = m_frame.document()->originIdentifierForPasteboard();
>Index: Source/WebCore/editing/cocoa/HTMLConverter.h
>===================================================================
>--- Source/WebCore/editing/cocoa/HTMLConverter.h	(revision 239063)
>+++ Source/WebCore/editing/cocoa/HTMLConverter.h	(working copy)
>@@ -30,9 +30,11 @@ OBJC_CLASS NSAttributedString;
> namespace WebCore {
>     
> class Range;
>+class VisibleSelection;
> 
> enum class IncludeImagesInAttributedString { Yes, No };
>-    
>+
>+NSAttributedString *attributedStringFromSelection(const VisibleSelection&);
> WEBCORE_EXPORT NSAttributedString *attributedStringFromRange(Range&);
> #if !PLATFORM(IOS_FAMILY)
> WEBCORE_EXPORT NSAttributedString *editingAttributedStringFromRange(Range&, IncludeImagesInAttributedString = IncludeImagesInAttributedString::Yes);
>Index: Source/WebCore/editing/cocoa/HTMLConverter.mm
>===================================================================
>--- Source/WebCore/editing/cocoa/HTMLConverter.mm	(revision 239063)
>+++ Source/WebCore/editing/cocoa/HTMLConverter.mm	(working copy)
>@@ -61,6 +61,7 @@
> #import "StyleProperties.h"
> #import "StyledElement.h"
> #import "TextIterator.h"
>+#import "VisibleSelection.h"
> #import <objc/runtime.h>
> #import <pal/spi/cocoa/NSAttributedStringSPI.h>
> #import <wtf/ASCIICType.h>
>@@ -336,7 +337,7 @@ public:
>     RefPtr<CSSValue> computedStylePropertyForElement(Element&, CSSPropertyID);
>     RefPtr<CSSValue> inlineStylePropertyForElement(Element&, CSSPropertyID);
> 
>-    Node* cacheAncestorsOfStartToBeConverted(const Range&);
>+    Node* cacheAncestorsOfStartToBeConverted(const Position&, const Position&);
>     bool isAncestorsOfStartToBeConverted(Node& node) const { return m_ancestorsUnderCommonAncestor.contains(&node); }
> 
> private:
>@@ -359,13 +360,14 @@ + (void)document:(NSObject **)outDocumen
> 
> class HTMLConverter {
> public:
>-    HTMLConverter(Range&);
>+    HTMLConverter(const Position&, const Position&);
>     ~HTMLConverter();
>     
>     NSAttributedString* convert();
>     
> private:
>-    Ref<Range> m_range;
>+    Position m_start;
>+    Position m_end;
>     DocumentLoader* m_dataSource;
>     
>     HashMap<RefPtr<Element>, RetainPtr<NSDictionary>> m_attributesForElements;
>@@ -434,8 +436,9 @@ private:
>     void _adjustTrailingNewline();
> };
> 
>-HTMLConverter::HTMLConverter(Range& range)
>-    : m_range(range)
>+HTMLConverter::HTMLConverter(const Position& start, const Position& end)
>+    : m_start(start)
>+    , m_end(end)
>     , m_dataSource(nullptr)
> {
>     _attrStr = [[NSMutableAttributedString alloc] init];
>@@ -481,7 +484,7 @@ HTMLConverter::~HTMLConverter()
> 
> NSAttributedString *HTMLConverter::convert()
> {
>-    Node* commonAncestorContainer = _caches->cacheAncestorsOfStartToBeConverted(m_range);
>+    Node* commonAncestorContainer = _caches->cacheAncestorsOfStartToBeConverted(m_start, m_end);
>     ASSERT(commonAncestorContainer);
> 
>     m_dataSource = commonAncestorContainer->document().frame()->loader().documentLoader();
>@@ -2197,13 +2200,13 @@ void HTMLConverter::_processText(Charact
>     String originalString = characterData.data();
>     unsigned startOffset = 0;
>     unsigned endOffset = originalString.length();
>-    if (&characterData == &m_range->startContainer()) {
>-        startOffset = m_range->startOffset();
>+    if (&characterData == m_start.containerNode()) {
>+        startOffset = m_start.offsetInContainerNode();
>         _domRangeStartIndex = [_attrStr length];
>         _flags.reachedStart = YES;
>     }
>-    if (&characterData == &m_range->endContainer()) {
>-        endOffset = m_range->endOffset();
>+    if (&characterData == m_end.containerNode()) {
>+        endOffset = m_end.offsetInContainerNode();
>         _flags.reachedEnd = YES;
>     }
>     if ((startOffset > 0 || endOffset < originalString.length()) && endOffset >= startOffset)
>@@ -2277,13 +2280,13 @@ void HTMLConverter::_traverseNode(Node& 
>     unsigned endOffset = UINT_MAX;
>     bool isStart = false;
>     bool isEnd = false;
>-    if (&node == &m_range->startContainer()) {
>-        startOffset = m_range->startOffset();
>+    if (&node == m_start.containerNode()) {
>+        startOffset = m_start.offsetInContainerNode();
>         isStart = true;
>         _flags.reachedStart = YES;
>     }
>-    if (&node == &m_range->endContainer()) {
>-        endOffset = m_range->endOffset();
>+    if (&node == m_end.containerNode()) {
>+        endOffset = m_end.offsetInContainerNode();
>         isEnd = true;
>     }
>     
>@@ -2338,13 +2341,13 @@ void HTMLConverter::_traverseFooterNode(
>     unsigned endOffset = UINT_MAX;
>     bool isStart = false;
>     bool isEnd = false;
>-    if (&element == &m_range->startContainer()) {
>-        startOffset = m_range->startOffset();
>+    if (&element == m_start.containerNode()) {
>+        startOffset = m_start.offsetInContainerNode();
>         isStart = true;
>         _flags.reachedStart = YES;
>     }
>-    if (&element == &m_range->endContainer()) {
>-        endOffset = m_range->endOffset();
>+    if (&element == m_end.containerNode()) {
>+        endOffset = m_end.offsetInContainerNode();
>         isEnd = true;
>     }
>     
>@@ -2379,10 +2382,10 @@ void HTMLConverter::_adjustTrailingNewli
>         [_attrStr replaceCharactersInRange:NSMakeRange(textLength, 0) withString:@"\n"];
> }
> 
>-Node* HTMLConverterCaches::cacheAncestorsOfStartToBeConverted(const Range& range)
>+Node* HTMLConverterCaches::cacheAncestorsOfStartToBeConverted(const Position& start, const Position& end)
> {
>-    Node* commonAncestor = range.commonAncestorContainer();
>-    Node* ancestor = &range.startContainer();
>+    Node* commonAncestor = Range::commonAncestorContainer(start.containerNode(), end.containerNode());
>+    Node* ancestor = start.containerNode();
> 
>     while (ancestor) {
>         m_ancestorsUnderCommonAncestor.add(ancestor);
>@@ -2453,7 +2456,15 @@ namespace WebCore {
> // This function supports more HTML features than the editing variant below, such as tables.
> NSAttributedString *attributedStringFromRange(Range& range)
> {
>-    HTMLConverter converter(range);
>+    HTMLConverter converter(range.startPosition(), range.endPosition());
>+    return converter.convert();
>+}
>+
>+NSAttributedString *attributedStringFromSelection(const VisibleSelection& selection)
>+{
>+    auto range = selection.toNormalizedRange();
>+    ASSERT(range);
>+    HTMLConverter converter(range->startPosition(), range->endPosition());
>     return converter.convert();
> }
>

Flags:

darin: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 192613: 357099