Add a new variant of serializePreservingVisualAppearance which takes VisibleSelection
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 2 Oct 2018 01:59:52 +0000 (01:59 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 2 Oct 2018 01:59:52 +0000 (01:59 +0000)
https://bugs.webkit.org/show_bug.cgi?id=190108

Reviewed by Wenson Hsieh.

Source/WebCore:

Added a version of serializePreservingVisualAppearance which takes VisibleSelection so that we can avoid creating
a range simply to get the first node and the end node of the selection later. This simple change also fixes a bug
demonstrated in editing/pasteboard/paste-table-003.html.

Test: editing/pasteboard/paste-table-003.html

* editing/cocoa/EditorCocoa.mm:
(WebCore::Editor::selectionInHTMLFormat): Adopt the new variant.
* editing/gtk/EditorGtk.cpp:
(WebCore::Editor::writeSelectionToPasteboard): Ditto.
* editing/markup.cpp:
(WebCore::serializePreservingVisualAppearance): Added.
* editing/markup.h:
* editing/wpe/EditorWPE.cpp:
(WebCore::Editor::writeSelectionToPasteboard): Ditto.
* loader/archive/cf/LegacyWebArchive.cpp:
(WebCore::LegacyWebArchive::createFromSelection): Ditto.
* platform/win/PasteboardWin.cpp:
(WebCore::Pasteboard::writeSelection): Ditto.

Source/WebKit:

Adopt the new variant which directly takes VisibleSelection.

* WebProcess/WebCoreSupport/gtk/WebEditorClientGtk.cpp:
(WebKit::WebEditorClient::updateGlobalSelection):

LayoutTests:

Rebaselined the test since the bug that interchange new lines are inserted in the last table cell is fixed.
Also updated the description in the test to reflect this change.

* editing/pasteboard/paste-table-003-expected.txt:
* editing/pasteboard/paste-table-003.html:

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

13 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/pasteboard/paste-table-003-expected.txt
LayoutTests/editing/pasteboard/paste-table-003.html
Source/WebCore/ChangeLog
Source/WebCore/editing/cocoa/EditorCocoa.mm
Source/WebCore/editing/gtk/EditorGtk.cpp
Source/WebCore/editing/markup.cpp
Source/WebCore/editing/markup.h
Source/WebCore/editing/wpe/EditorWPE.cpp
Source/WebCore/loader/archive/cf/LegacyWebArchive.cpp
Source/WebCore/platform/win/PasteboardWin.cpp
Source/WebKit/ChangeLog
Source/WebKit/WebProcess/WebCoreSupport/gtk/WebEditorClientGtk.cpp

index b5e6943..bb62af4 100644 (file)
@@ -1,3 +1,16 @@
+2018-10-01  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Add a new variant of serializePreservingVisualAppearance which takes VisibleSelection
+        https://bugs.webkit.org/show_bug.cgi?id=190108
+
+        Reviewed by Wenson Hsieh.
+
+        Rebaselined the test since the bug that interchange new lines are inserted in the last table cell is fixed.
+        Also updated the description in the test to reflect this change.
+
+        * editing/pasteboard/paste-table-003-expected.txt:
+        * editing/pasteboard/paste-table-003.html:
+
 2018-10-01  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rebaseline tests after r236632.
index d9690c7..0524ea4 100644 (file)
@@ -5,13 +5,13 @@ EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotificatio
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 0 of #text > DIV > DIV > BODY > HTML > #document to 9 of #text > DIV > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > BODY > HTML > #document toDOMRange:range from 2 of TD > TR > TBODY > TABLE > DIV > DIV > BODY > HTML > #document to 2 of TD > TR > TBODY > TABLE > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > BODY > HTML > #document toDOMRange:range from 3 of #text > TD > TR > TBODY > TABLE > DIV > DIV > BODY > HTML > #document to 3 of #text > TD > TR > TBODY > TABLE > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
-This tests pasting a table to replace some text. It demonstrates two bugs: 1) selecting a table without also selecting the line break after programmatically is impossible when its followed by a div because of the way DOM positions are mapped to visible positions, 2) pasting content that ends in a table places the caret in the last table cell instead of after the table, so the interchange newline is placed in the last table cell and not after the table.
+This tests pasting a table to replace some text.
+It demonstrates a bug: selecting a table without also selecting the line break after programmatically is impossible when its followed by a div because of the way DOM positions are mapped to visible positions.
 
 one    two
 one    two
-
 execCopyCommand: <table id="test"> <tbody><tr><td>one</td><td>two</td></tr></tbody></table> <div id="target">replaceme</div>
-execPasteCommand: <table id="test"> <tbody><tr><td>one</td><td>two</td></tr></tbody></table> <div id="target"><table id="test"><tbody><tr><td>one</td><td>two<br><br></td></tr></tbody></table></div>
+execPasteCommand: <table id="test"> <tbody><tr><td>one</td><td>two</td></tr></tbody></table> <div id="target"><table id="test"><tbody><tr><td>one</td><td>two</td></tr></tbody></table></div>
index 38233ab..a867a18 100644 (file)
@@ -34,7 +34,9 @@ function editingTest() {
 <title>Editing Test</title> 
 </head> 
 <body>
-<p>This tests pasting a table to replace some text.  <b>It demonstrates two bugs: 1) selecting a table without also selecting the line break after programmatically is impossible when its followed by a div because of the way DOM positions are mapped to visible positions, 2) pasting content that ends in a table places the caret in the last table cell instead of after the table, so the interchange newline is placed in the last table cell and not after the table.</b></p>
+<p>This tests pasting a table to replace some text.<br>
+It demonstrates a bug: selecting a table without also selecting the line break after programmatically is impossible
+when its followed by a div because of the way DOM positions are mapped to visible positions.</p>
 <div id="root" contenteditable>
 <table id="test">
 <tbody><tr><td>one</td><td>two</td></tr></tbody></table>
index 083cab3..4c59b64 100644 (file)
@@ -1,3 +1,30 @@
+2018-10-01  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Add a new variant of serializePreservingVisualAppearance which takes VisibleSelection
+        https://bugs.webkit.org/show_bug.cgi?id=190108
+
+        Reviewed by Wenson Hsieh.
+
+        Added a version of serializePreservingVisualAppearance which takes VisibleSelection so that we can avoid creating
+        a range simply to get the first node and the end node of the selection later. This simple change also fixes a bug
+        demonstrated in editing/pasteboard/paste-table-003.html.
+
+        Test: editing/pasteboard/paste-table-003.html
+
+        * editing/cocoa/EditorCocoa.mm:
+        (WebCore::Editor::selectionInHTMLFormat): Adopt the new variant.
+        * editing/gtk/EditorGtk.cpp:
+        (WebCore::Editor::writeSelectionToPasteboard): Ditto.
+        * editing/markup.cpp:
+        (WebCore::serializePreservingVisualAppearance): Added.
+        * editing/markup.h:
+        * editing/wpe/EditorWPE.cpp:
+        (WebCore::Editor::writeSelectionToPasteboard): Ditto.
+        * loader/archive/cf/LegacyWebArchive.cpp:
+        (WebCore::LegacyWebArchive::createFromSelection): Ditto.
+        * platform/win/PasteboardWin.cpp:
+        (WebCore::Pasteboard::writeSelection): Ditto.
+
 2018-10-01  Alex Christensen  <achristensen@webkit.org>
 
         Don't read from WebCore's bundle for IDNScriptWhiteList
index ab58923..af61f36 100644 (file)
@@ -75,9 +75,7 @@ static RefPtr<SharedBuffer> archivedDataForAttributedString(NSAttributedString *
 
 String Editor::selectionInHTMLFormat()
 {
-    if (auto range = selectedRange())
-        return serializePreservingVisualAppearance(*range, nullptr, AnnotateForInterchange::Yes, ConvertBlocksToInlines::No, ResolveURLs::YesExcludingLocalFileURLsForPrivacy);
-    return { };
+    return serializePreservingVisualAppearance(m_frame.selection().selection(), ResolveURLs::YesExcludingLocalFileURLsForPrivacy);
 }
 
 #if ENABLE(ATTACHMENT_ELEMENT)
index fc5e262..be5e2e3 100644 (file)
@@ -146,7 +146,7 @@ void Editor::writeSelectionToPasteboard(Pasteboard& pasteboard)
     PasteboardWebContent pasteboardContent;
     pasteboardContent.canSmartCopyOrDelete = canSmartCopyOrDelete();
     pasteboardContent.text = selectedTextForDataTransfer();
-    pasteboardContent.markup = serializePreservingVisualAppearance(*selectedRange(), nullptr, AnnotateForInterchange::Yes, ConvertBlocksToInlines::No, ResolveURLs::YesExcludingLocalFileURLsForPrivacy);
+    pasteboardContent.markup = serializePreservingVisualAppearance(m_frame.selection().selection(), ResolveURLs::YesExcludingLocalFileURLsForPrivacy);
     pasteboard.write(pasteboardContent);
 }
 
index f94fb31..bf2a126 100644 (file)
@@ -852,6 +852,13 @@ String serializePreservingVisualAppearance(const Range& range, Vector<Node*>* no
     return serializePreservingVisualAppearanceInternal(range.startPosition(), range.endPosition(), nodes, annotate, convertBlocksToInlines, urlsToReslve, MSOListMode::DoNotPreserve);
 }
 
+String serializePreservingVisualAppearance(const VisibleSelection& selection, ResolveURLs resolveURLs, Vector<Node*>* nodes)
+{
+    return serializePreservingVisualAppearanceInternal(selection.start(), selection.end(), nodes,
+        AnnotateForInterchange::Yes, ConvertBlocksToInlines::No, resolveURLs, MSOListMode::DoNotPreserve);
+}
+
+
 static bool shouldPreserveMSOLists(const String& markup)
 {
     if (!markup.startsWith("<html xmlns:"))
index 292eaed..b550a8f 100644 (file)
@@ -46,6 +46,7 @@ class Node;
 class Page;
 class QualifiedName;
 class Range;
+class VisibleSelection;
 
 void replaceSubresourceURLs(Ref<DocumentFragment>&&, HashMap<AtomicString, AtomicString>&&);
 void removeSubresourceURLAttributes(Ref<DocumentFragment>&&, WTF::Function<bool(const URL&)> shouldRemoveURL);
@@ -70,6 +71,7 @@ ExceptionOr<void> replaceChildrenWithFragment(ContainerNode&, Ref<DocumentFragme
 enum class ResolveURLs : uint8_t { No, Yes, YesExcludingLocalFileURLsForPrivacy };
 enum class ConvertBlocksToInlines : uint8_t { No, Yes };
 WEBCORE_EXPORT String serializePreservingVisualAppearance(const Range&, Vector<Node*>* = nullptr, AnnotateForInterchange = AnnotateForInterchange::No, ConvertBlocksToInlines = ConvertBlocksToInlines::No, ResolveURLs = ResolveURLs::No);
+String serializePreservingVisualAppearance(const VisibleSelection&, ResolveURLs = ResolveURLs::No, Vector<Node*>* = nullptr);
 
 enum class SerializedNodes : uint8_t { SubtreeIncludingNode, SubtreesOfChildren };
 enum class SerializationSyntax : uint8_t { HTML, XML };
index a08efb8..2dcf8c3 100644 (file)
@@ -64,7 +64,7 @@ void Editor::writeSelectionToPasteboard(Pasteboard& pasteboard)
 {
     PasteboardWebContent pasteboardContent;
     pasteboardContent.text = selectedTextForDataTransfer();
-    pasteboardContent.markup = serializePreservingVisualAppearance(*selectedRange(), nullptr, AnnotateForInterchange::Yes, ConvertBlocksToInlines::No, ResolveURLs::YesExcludingLocalFileURLsForPrivacy);
+    pasteboardContent.markup = serializePreservingVisualAppearance(m_frame.selection().selection(), ResolveURLs::YesExcludingLocalFileURLsForPrivacy);
     pasteboard.write(pasteboardContent);
 }
 
index 7eb06da..e874892 100644 (file)
@@ -554,8 +554,7 @@ RefPtr<LegacyWebArchive> LegacyWebArchive::createFromSelection(Frame* frame)
     builder.append(documentTypeString(*document));
 
     Vector<Node*> nodeList;
-    if (auto selectionRange = frame->selection().toNormalizedRange())
-        builder.append(serializePreservingVisualAppearance(*selectionRange, &nodeList, AnnotateForInterchange::Yes));
+    builder.append(serializePreservingVisualAppearance(frame->selection().selection(), ResolveURLs::No, &nodeList));
 
     auto archive = create(builder.toString(), *frame, nodeList, nullptr);
     
index 1f5c408..0699799 100644 (file)
@@ -469,7 +469,8 @@ void Pasteboard::writeSelection(Range& selectedRange, bool canSmartCopyOrDelete,
     // Put CF_HTML format on the pasteboard 
     if (::OpenClipboard(m_owner)) {
         Vector<char> data;
-        markupToCFHTML(serializePreservingVisualAppearance(selectedRange, nullptr, AnnotateForInterchange::Yes),
+        // FIXME: Use ResolveURLs::YesExcludingLocalFileURLsForPrivacy.
+        markupToCFHTML(serializePreservingVisualAppearance(frame.selection().selection()),
             selectedRange.startContainer().document().url().string(), data);
         HGLOBAL cbData = createGlobalData(data);
         if (!::SetClipboardData(HTMLClipboardFormat, cbData))
index 3789c1d..a710c75 100644 (file)
@@ -1,3 +1,15 @@
+2018-09-30  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Add a new variant of serializePreservingVisualAppearance which takes VisibleSelection
+        https://bugs.webkit.org/show_bug.cgi?id=190108
+
+        Reviewed by Wenson Hsieh.
+
+        Adopt the new variant which directly takes VisibleSelection.
+
+        * WebProcess/WebCoreSupport/gtk/WebEditorClientGtk.cpp:
+        (WebKit::WebEditorClient::updateGlobalSelection):
+
 2018-10-01  Andy Estes  <aestes@apple.com>
 
         [watchOS] Adopt NSURLSessionCompanionProxyPreference
index 062f376..0d8d018 100644 (file)
@@ -126,7 +126,7 @@ void WebEditorClient::updateGlobalSelection(Frame* frame)
     PasteboardWebContent pasteboardContent;
     pasteboardContent.canSmartCopyOrDelete = false;
     pasteboardContent.text = range->text();
-    pasteboardContent.markup = serializePreservingVisualAppearance(*range, nullptr, AnnotateForInterchange::Yes, ConvertBlocksToInlines::No, ResolveURLs::YesExcludingLocalFileURLsForPrivacy);
+    pasteboardContent.markup = serializePreservingVisualAppearance(frame->selection().selection(), ResolveURLs::YesExcludingLocalFileURLsForPrivacy);
     Pasteboard::createForGlobalSelection()->write(pasteboardContent);
 }