Skip to content
Permalink
Browse files
2011-05-20 Ryosuke Niwa <rniwa@webkit.org>
        Reviewed by Enrica Casucci.

        Wrap copied contents by one style span instead of two
        https://bugs.webkit.org/show_bug.cgi?id=60988

        Rebaselined tests due to the change in how WebKit preserves style in copy and paste.

        * editing/pasteboard/4930986-2-expected.txt: Whitespace change.
        * editing/pasteboard/5065605-expected.txt: No longer adds redundant inline style declaration.
        * editing/pasteboard/paste-4039777-fix-expected.txt: Progression; Now we hit the list merging logic
        in ReplaceSelectionCommand: isStyleSpan(refNode.get()) && isListElement(refNode->firstChild()).
        * editing/pasteboard/paste-list-001-expected.txt: Ditto.
        * editing/pasteboard/paste-text-011-expected.txt: An extra style span was added.
        * editing/pasteboard/paste-text-012-expected.txt: Ditto.
        * editing/pasteboard/smart-paste-003-trailing-whitespace-expected.txt: No longer adds redundant style span.
        * platform/chromium-win/editing/pasteboard/paste-text-003-expected.txt: No longer adds empty anonymous nodes.
        * platform/chromium-win/editing/pasteboard/paste-text-011-expected.txt: Ditto.
        * platform/gtk/editing/pasteboard/paste-text-003-expected.txt: Ditto.
        * platform/mac/editing/pasteboard/paste-text-003-expected.txt: Ditto.
        * platform/qt/editing/pasteboard/paste-text-003-expected.txt: Ditto.
2011-05-20  Ryosuke Niwa  <rniwa@webkit.org>

        Reviewed by Enrica Casucci.

        Wrap copied contents by one style span instead of two
        https://bugs.webkit.org/show_bug.cgi?id=60988

        Replaced sourceDocumentStyleSpan and copiedRangeStyleSpan by one wrapping style span. Instead
        of wrapping the copied contents by user-applied style and document default style in serialization,
        take the difference with the document default's style in paste code.

        This will dramatically simplify our copy and paste code and pave a way to fix the bug 60914.

        No new tests because copy & paste is tested by existing layout tests.

        * editing/EditingStyle.cpp:
        (WebCore::EditingStyle::prepareToApplyAt): Remove the color property if RGBA values of color
        matches that of the computed style at the specified position.
        * editing/ReplaceSelectionCommand.cpp:
        (WebCore::ReplaceSelectionCommand::handleStyleSpans): Replaced sourceDocumentStyleSpan and
        copiedRangeStyleSpan by wrappingStyleSpan. When pasting as a quotation, compare style against
        the document's default style to avoid keeping the document default style (tested by
        editing/pasteboard/4930986-3.html).
        * editing/ReplaceSelectionCommand.h:
        * editing/markup.cpp:
        (WebCore::createMarkup): Only use one style span to wrap the serialized contents.


Canonical link: https://commits.webkit.org/76586@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@86983 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
rniwa committed May 20, 2011
1 parent 718179d commit 5e448b4e8a55b32752c2e78ed26533cb1146748e
Showing 18 changed files with 125 additions and 151 deletions.
@@ -1,3 +1,26 @@
2011-05-20 Ryosuke Niwa <rniwa@webkit.org>

Reviewed by Enrica Casucci.

Wrap copied contents by one style span instead of two
https://bugs.webkit.org/show_bug.cgi?id=60988

Rebaselined tests due to the change in how WebKit preserves style in copy and paste.

* editing/pasteboard/4930986-2-expected.txt: Whitespace change.
* editing/pasteboard/5065605-expected.txt: No longer adds redundant inline style declaration.
* editing/pasteboard/paste-4039777-fix-expected.txt: Progression; Now we hit the list merging logic
in ReplaceSelectionCommand: isStyleSpan(refNode.get()) && isListElement(refNode->firstChild()).
* editing/pasteboard/paste-list-001-expected.txt: Ditto.
* editing/pasteboard/paste-text-011-expected.txt: An extra style span was added.
* editing/pasteboard/paste-text-012-expected.txt: Ditto.
* editing/pasteboard/smart-paste-003-trailing-whitespace-expected.txt: No longer adds redundant style span.
* platform/chromium-win/editing/pasteboard/paste-text-003-expected.txt: No longer adds empty anonymous nodes.
* platform/chromium-win/editing/pasteboard/paste-text-011-expected.txt: Ditto.
* platform/gtk/editing/pasteboard/paste-text-003-expected.txt: Ditto.
* platform/mac/editing/pasteboard/paste-text-003-expected.txt: Ditto.
* platform/qt/editing/pasteboard/paste-text-003-expected.txt: Ditto.

2011-05-20 Justin Schuh <jschuh@chromium.org>

Unreviewed.
@@ -1,2 +1,2 @@
This tests to make sure that content that is colored by the user is pasted with that color during a Paste as Quotation.
<blockquote><span class="Apple-style-span" style="color: red; ">This text should be red (it should be wrapped in a style span).</span></blockquote>
<blockquote><span class="Apple-style-span" style="color: red;">This text should be red (it should be wrapped in a style span).</span></blockquote>
@@ -21,7 +21,7 @@ EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotificatio
EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 0 of DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 24 of #text > FONT > DIV > FONT > DIV > DIV > BODY > HTML > #document to 24 of #text > FONT > DIV > FONT > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 24 of #text > FONT > DIV > SPAN > FONT > DIV > DIV > BODY > HTML > #document to 24 of #text > FONT > DIV > SPAN > FONT > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
@@ -47,9 +47,8 @@ This tests for a bug where text copied with Select All + Copy would lose its col
| class="Apple-style-span"
| color="#ff0000"
| "This text should be red."
| <div>
| style="color: rgb(0, 0, 0); "
| <font>
| class="Apple-style-span"
| color="#ff0000"
| "This text should be red.<#selection-caret>"
| <div>
| <font>
| class="Apple-style-span"
| color="#ff0000"
| "This text should be red.<#selection-caret>"
@@ -5,7 +5,7 @@ EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotificatio
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 0 of #text > LI > UL > DIV > DIV > BODY > HTML > #document to 14 of #text > LI > UL > DIV > DIV > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 1 of #text > LI > UL > DIV > LI > UL > DIV > DIV > BODY > HTML > #document to 1 of #text > LI > UL > DIV > LI > UL > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 1 of #text > LI > UL > DIV > UL > DIV > DIV > BODY > HTML > #document to 1 of #text > LI > UL > DIV > UL > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
@@ -51,20 +51,19 @@ Actual result:
"
| <ul>
| style="text-align:right;"
| <li>
| <ul>
| style="text-align: right; "
| <li>
| "A"
| <div>
| <ul>
| style="text-align: right; "
| <li>
| "A"
| <div>
| <ul>
| <li>
| <a>
| href=""
| "B"
| " "
| <br>
| "C<#selection-caret>"
| <a>
| href=""
| "B"
| " "
| <br>
| "C<#selection-caret>"
| <div>
| <ul>
| "
@@ -7,7 +7,7 @@ EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotificatio
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 21 of #text > LI > OL > DIV > BODY > HTML > #document to 21 of #text > LI > OL > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 21 of #text > LI > OL > DIV > BODY > HTML > #document to 21 of #text > LI > OL > DIV > BODY > HTML > #document toDOMRange:range from 21 of #text > LI > OL > LI > OL > DIV > BODY > HTML > #document to 21 of #text > LI > OL > LI > OL > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 21 of #text > LI > OL > DIV > BODY > HTML > #document to 21 of #text > LI > OL > DIV > BODY > HTML > #document toDOMRange:range from 21 of #text > LI > OL > DIV > BODY > HTML > #document to 21 of #text > LI > OL > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
@@ -34,12 +34,11 @@ EDITING DELEGATE: webViewDidEndEditing:WebViewDidEndEditingNotification
"
| <li>
| "I should be number 3."
| <ol>
| <li>
| "I should be number 1."
| <li>
| <li>
| "I should be number 3.<#selection-caret>"
| <li>
| "I should be number 1."
| <li>
| <li>
| "I should be number 3.<#selection-caret>"
| "
"
| "
@@ -6,7 +6,7 @@ EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotificatio
EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 0 of P > BODY > HTML > #document to 0 of P > BODY > HTML > #document givenAction:WebViewInsertActionPasted
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 5 of #text > B > FONT > P > B > FONT > P > BODY > HTML > #document to 5 of #text > B > FONT > P > B > FONT > P > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 5 of #text > B > FONT > P > SPAN > B > FONT > P > BODY > HTML > #document to 5 of #text > B > FONT > P > SPAN > B > FONT > P > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
@@ -30,18 +30,19 @@ The content was inserted at the start of the block instead of the end. This test
| <font>
| face="Monaco"
| <b>
| <p>
| <span>
| class="Apple-style-span"
| style="font-family: Times; font-weight: normal; "
| <font>
| face="Monaco"
| <b>
| "hello"
| <p>
| style="font-family: Times; font-weight: normal; "
| <font>
| face="Monaco"
| <b>
| "there<#selection-caret>"
| <p>
| <font>
| face="Monaco"
| <b>
| "hello"
| <p>
| <font>
| face="Monaco"
| <b>
| "there<#selection-caret>"
| "


@@ -6,4 +6,4 @@ foo
foo

execCopyCommand: <div id="test" class="editing"><div><blockquote>foo</blockquote></div></div> <div class="editing"></div>
execPasteCommand: <div id="test" class="editing"><div><blockquote>foo</blockquote></div></div> <div class="editing"><div id="test" class="editing" style="border-top-width: 2px; border-right-width: 2px; border-bottom-width: 2px; border-left-width: 2px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: red; border-right-color: red; border-bottom-color: red; border-left-color: red; padding-top: 12px; padding-right: 12px; padding-bottom: 12px; padding-left: 12px; font-size: 24px; "><div><blockquote>foo</blockquote></div><div><br></div></div></div>
execPasteCommand: <div id="test" class="editing"><div><blockquote>foo</blockquote></div></div> <div class="editing"><span class="Apple-style-span" style="font-size: medium; "><div id="test" class="editing" style="border-top-width: 2px; border-right-width: 2px; border-bottom-width: 2px; border-left-width: 2px; border-top-style: solid; border-right-style: solid; border-bottom-style: solid; border-left-style: solid; border-top-color: red; border-right-color: red; border-bottom-color: red; border-left-color: red; padding-top: 12px; padding-right: 12px; padding-bottom: 12px; padding-left: 12px; font-size: 24px; "><div><blockquote>foo</blockquote></div><div><br></div></div></span></div>
@@ -9,4 +9,4 @@ A space should be added between the preexisting word and the word that's pasted.
Actual result

execCopyCommand: <div id="test" class="editing" style="font-size: 24px;"> hello world </div>
execPasteCommand: <div id="test" class="editing" style="font-size: 24px;"> hello&nbsp;hello<span class="Apple-style-span" style="font-size: 24px; ">&nbsp;</span>world</div>
execPasteCommand: <div id="test" class="editing" style="font-size: 24px;"> hello&nbsp;hello&nbsp;world</div>
@@ -118,7 +118,6 @@ layer at (0,0) size 800x600
RenderBlock {DIV} at (14,42) size 756x56 [border: (2px solid #FF0000)]
RenderText {#text} at (14,14) size 431x27
text run at (14,14) width 431: "Which taken at the flood leads on to fortune."
RenderBlock (anonymous) at (14,98) size 756x0
RenderBlock {DIV} at (14,98) size 756x252 [border: (2px solid #FF0000)]
RenderBlock (anonymous) at (14,14) size 728x0
RenderBlock {DIV} at (14,14) size 728x56 [border: (2px solid #FF0000)]
@@ -130,7 +129,6 @@ layer at (0,0) size 800x600
RenderBlock {DIV} at (14,70) size 728x56 [border: (2px solid #FF0000)]
RenderText {#text} at (14,14) size 431x27
text run at (14,14) width 431: "Which taken at the flood leads on to fortune."
RenderBlock (anonymous) at (14,126) size 728x0
RenderBlock {DIV} at (14,126) size 728x112 [border: (2px solid #FF0000)]
RenderBlock (anonymous) at (14,14) size 700x28
RenderText {#text} at (0,0) size 78x27
@@ -6,7 +6,7 @@ EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotificatio
EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 0 of P > BODY > HTML > #document to 0 of P > BODY > HTML > #document givenAction:WebViewInsertActionPasted
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 5 of #text > B > FONT > P > B > FONT > P > BODY > HTML > #document to 5 of #text > B > FONT > P > B > FONT > P > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 5 of #text > B > FONT > P > SPAN > B > FONT > P > BODY > HTML > #document to 5 of #text > B > FONT > P > SPAN > B > FONT > P > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
@@ -30,18 +30,19 @@ The content was inserted at the start of the block instead of the end. This test
| <font>
| face="Monaco"
| <b>
| <p>
| <span>
| class="Apple-style-span"
| style="font-family: 'times new roman'; font-weight: normal; "
| <font>
| face="Monaco"
| <b>
| "hello"
| <p>
| style="font-family: 'times new roman'; font-weight: normal; "
| <font>
| face="Monaco"
| <b>
| "there<#selection-caret>"
| <p>
| <font>
| face="Monaco"
| <b>
| "hello"
| <p>
| <font>
| face="Monaco"
| <b>
| "there<#selection-caret>"
| "


@@ -118,7 +118,6 @@ layer at (0,0) size 800x600
RenderBlock {DIV} at (14,42) size 756x56 [border: (2px solid #FF0000)]
RenderText {#text} at (14,14) size 434x28
text run at (14,14) width 434: "Which taken at the flood leads on to fortune."
RenderBlock (anonymous) at (14,98) size 756x0
RenderBlock {DIV} at (14,98) size 756x252 [border: (2px solid #FF0000)]
RenderBlock (anonymous) at (14,14) size 728x0
RenderBlock {DIV} at (14,14) size 728x56 [border: (2px solid #FF0000)]
@@ -130,7 +129,6 @@ layer at (0,0) size 800x600
RenderBlock {DIV} at (14,70) size 728x56 [border: (2px solid #FF0000)]
RenderText {#text} at (14,14) size 434x28
text run at (14,14) width 434: "Which taken at the flood leads on to fortune."
RenderBlock (anonymous) at (14,126) size 728x0
RenderBlock {DIV} at (14,126) size 728x112 [border: (2px solid #FF0000)]
RenderBlock (anonymous) at (14,14) size 700x28
RenderText {#text} at (0,0) size 80x28
@@ -118,7 +118,6 @@ layer at (0,0) size 800x600
RenderBlock {DIV} at (14,42) size 756x56 [border: (2px solid #FF0000)]
RenderText {#text} at (14,14) size 434x28
text run at (14,14) width 434: "Which taken at the flood leads on to fortune."
RenderBlock (anonymous) at (14,98) size 756x0
RenderBlock {DIV} at (14,98) size 756x252 [border: (2px solid #FF0000)]
RenderBlock (anonymous) at (14,14) size 728x0
RenderBlock {DIV} at (14,14) size 728x56 [border: (2px solid #FF0000)]
@@ -130,7 +129,6 @@ layer at (0,0) size 800x600
RenderBlock {DIV} at (14,70) size 728x56 [border: (2px solid #FF0000)]
RenderText {#text} at (14,14) size 434x28
text run at (14,14) width 434: "Which taken at the flood leads on to fortune."
RenderBlock (anonymous) at (14,126) size 728x0
RenderBlock {DIV} at (14,126) size 728x112 [border: (2px solid #FF0000)]
RenderBlock (anonymous) at (14,14) size 700x28
RenderText {#text} at (0,0) size 80x28
@@ -118,7 +118,6 @@ layer at (0,0) size 800x600
RenderBlock {DIV} at (14,47) size 756x61 [border: (2px solid #FF0000)]
RenderText {#text} at (14,14) size 456x33
text run at (14,14) width 456: "Which taken at the flood leads on to fortune."
RenderBlock (anonymous) at (14,108) size 756x0
RenderBlock {DIV} at (14,108) size 756x272 [border: (2px solid #FF0000)]
RenderBlock (anonymous) at (14,14) size 728x0
RenderBlock {DIV} at (14,14) size 728x61 [border: (2px solid #FF0000)]
@@ -130,7 +129,6 @@ layer at (0,0) size 800x600
RenderBlock {DIV} at (14,75) size 728x61 [border: (2px solid #FF0000)]
RenderText {#text} at (14,14) size 456x33
text run at (14,14) width 456: "Which taken at the flood leads on to fortune."
RenderBlock (anonymous) at (14,136) size 728x0
RenderBlock {DIV} at (14,136) size 728x122 [border: (2px solid #FF0000)]
RenderBlock (anonymous) at (14,14) size 700x33
RenderText {#text} at (0,0) size 86x33
@@ -1,3 +1,30 @@
2011-05-20 Ryosuke Niwa <rniwa@webkit.org>

Reviewed by Enrica Casucci.

Wrap copied contents by one style span instead of two
https://bugs.webkit.org/show_bug.cgi?id=60988

Replaced sourceDocumentStyleSpan and copiedRangeStyleSpan by one wrapping style span. Instead
of wrapping the copied contents by user-applied style and document default style in serialization,
take the difference with the document default's style in paste code.

This will dramatically simplify our copy and paste code and pave a way to fix the bug 60914.

No new tests because copy & paste is tested by existing layout tests.

* editing/EditingStyle.cpp:
(WebCore::EditingStyle::prepareToApplyAt): Remove the color property if RGBA values of color
matches that of the computed style at the specified position.
* editing/ReplaceSelectionCommand.cpp:
(WebCore::ReplaceSelectionCommand::handleStyleSpans): Replaced sourceDocumentStyleSpan and
copiedRangeStyleSpan by wrappingStyleSpan. When pasting as a quotation, compare style against
the document's default style to avoid keeping the document default style (tested by
editing/pasteboard/4930986-3.html).
* editing/ReplaceSelectionCommand.h:
* editing/markup.cpp:
(WebCore::createMarkup): Only use one style span to wrap the serialized contents.

2011-05-20 Simon Fraser <simon.fraser@apple.com>

Reviewed by Sam Weinig.

0 comments on commit 5e448b4

Please sign in to comment.