Include style tags when pasting HTML content.

Include style tags when pasting HTML content.

Applications such as Excel put HTML content on the clipboard which
includes STYLE elements. Users expect the styles to remain when
pasting this content.

Chromium used to strip STYLE elements like WebKit. This change leaves
the STYLE elements in place like Firefox and Edge. This means pasted
content could change the formatting of the surrounding page, but
editors could work around this by processing STYLE tags.

BUG=121163
Committed: https://crrev.com/8ebfae3567b1bfe350286345bb0c338b9f354fe1
Cr-Commit-Position: refs/heads/master@{#403867}

[dominicc (has gone to gerrit), 2016-07-04 05:58:23]

dominicc@chromium.org changed reviewers:
+ esprehn@chromium.org

[dominicc (has gone to gerrit), 2016-07-04 06:00:57]

PTAL

FWIW I have developed an alternative patch which serializes the style rules into
attributes (to a point; still needs some work.) I'm starting to think simply
pasting the style elements is better because it's closer to what other browsers
do. (Everything is still a bit different, for example Chromium/WebKit includes
the HEAD element but Firefox strips it and reparents the styles, etc.)

[dominicc (has gone to gerrit), 2016-07-06 02:28:27]

dominicc@chromium.org changed reviewers:
+ yosin@chromium.org
- esprehn@chromium.org

[dominicc (has gone to gerrit), 2016-07-06 02:28:40]

PTAL

[yosin_UTC9, 2016-07-06 03:43:59]

yosin@chromium.org changed reviewers:
+ tkent@chromium.org, yosin@chromium.orgm

[yosin_UTC9, 2016-07-06 03:44:01]

lgtm

+tken@, for second opinion about gTest vs. layout test

https://codereview.chromium.org/2121613003/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/editing/pasteboard/paste-head-contents.html
(right):

https://codereview.chromium.org/2121613003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/editing/pasteboard/paste-head-contents.html:64:
</body>
OPTIONAL: It is better to convert w3c test harness.

https://codereview.chromium.org/2121613003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/core.gypi (right):

https://codereview.chromium.org/2121613003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/core.gypi:4049:
'editing/commands/ReplaceSelectionCommandTest.cpp',
(^_^)b

https://codereview.chromium.org/2121613003/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommandTest.cpp
(right):

https://codereview.chromium.org/2121613003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommandTest.cpp:24:
namespace {
You don't need to put TEST() in anonymous namespace.
Since gTest's namespace doesn't care about namespace.
Note: We should put class in anonymous namespace.

https://codereview.chromium.org/2121613003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommandTest.cpp:37:
fragment->parseHTML(
This is the reason why it is better to use gTest rather than layout-test.
We can assemble a HTML fragment by the code.

I used to be doing following magical thing.

    document.addEventListener('copy', event => {
        event.clipboardData.setData('text/html', '<span></span><span></span>');
        event.clipboardData.setData('text/plain', '');
        event.preventDefault();
    });
    // Put empty SPAN element with STYLE attribute into clipboard.
    document.execCommand('Copy');

Or, is it worth to do such magic to run layout test on Firefox and Edge?

[yosin_UTC9, 2016-07-06 04:04:40]

On 2016/07/06 at 03:44:01, Yosi_UTC9 wrote:
> lgtm
> 
> +tken@, for second opinion about gTest vs. layout test
> 
>
https://codereview.chromium.org/2121613003/diff/40001/third_party/WebKit/Layo...
> File
third_party/WebKit/LayoutTests/editing/pasteboard/paste-head-contents.html
(right):
> 
>
https://codereview.chromium.org/2121613003/diff/40001/third_party/WebKit/Layo...
> third_party/WebKit/LayoutTests/editing/pasteboard/paste-head-contents.html:64:
</body>
> OPTIONAL: It is better to convert w3c test harness.
> 
>
https://codereview.chromium.org/2121613003/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/core.gypi (right):
> 
>
https://codereview.chromium.org/2121613003/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/core.gypi:4049:
'editing/commands/ReplaceSelectionCommandTest.cpp',
> (^_^)b
> 
>
https://codereview.chromium.org/2121613003/diff/40001/third_party/WebKit/Sour...
> File
third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommandTest.cpp
(right):
> 
>
https://codereview.chromium.org/2121613003/diff/40001/third_party/WebKit/Sour...
>
third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommandTest.cpp:24:
namespace {
> You don't need to put TEST() in anonymous namespace.
> Since gTest's namespace doesn't care about namespace.
> Note: We should put class in anonymous namespace.
> 
>
https://codereview.chromium.org/2121613003/diff/40001/third_party/WebKit/Sour...
>
third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommandTest.cpp:37:
fragment->parseHTML(
> This is the reason why it is better to use gTest rather than layout-test.
> We can assemble a HTML fragment by the code.
> 
> I used to be doing following magical thing.
> 
>     document.addEventListener('copy', event => {
>         event.clipboardData.setData('text/html',
'<span></span><span></span>');
>         event.clipboardData.setData('text/plain', '');
>         event.preventDefault();
>     });
>     // Put empty SPAN element with STYLE attribute into clipboard.
>     document.execCommand('Copy');
> 
> Or, is it worth to do such magic to run layout test on Firefox and Edge?

Sorry, I'm wrong. document.execCommand('Copy') doesn't work since it requires
user interaction.
So, we should use gTetst.

[tkent, 2016-07-06 04:05:20]

https://codereview.chromium.org/2121613003/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/editing/pasteboard/paste-head-contents.html
(right):

https://codereview.chromium.org/2121613003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/editing/pasteboard/paste-head-contents.html:64:
</body>
On 2016/07/06 at 03:44:00, Yosi_UTC9 wrote:
> OPTIONAL: It is better to convert w3c test harness.

IMO, updating a test behavior and switching test harness should not happen in a
single CL.

https://codereview.chromium.org/2121613003/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommandTest.cpp
(right):

https://codereview.chromium.org/2121613003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommandTest.cpp:19:
#include <gtest/gtest.h>
We should not use <> for gtest.h because it's not a system header.

#include "testing/gtest/include/gtest/gtest.h"

https://codereview.chromium.org/2121613003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommandTest.cpp:37:
fragment->parseHTML(
On 2016/07/06 at 03:44:01, Yosi_UTC9 wrote:
> This is the reason why it is better to use gTest rather than layout-test.
> We can assemble a HTML fragment by the code.
> 
> I used to be doing following magical thing.
> 
>     document.addEventListener('copy', event => {
>         event.clipboardData.setData('text/html',
'<span></span><span></span>');
>         event.clipboardData.setData('text/plain', '');
>         event.preventDefault();
>     });
>     // Put empty SPAN element with STYLE attribute into clipboard.
>     document.execCommand('Copy');
> 
> Or, is it worth to do such magic to run layout test on Firefox and Edge?

The copy command needs a user gesture.  So it's impossible to write an
interoperable automated JS test.

[yosin_UTC9, 2016-07-06 04:05:47]

https://codereview.chromium.org/2121613003/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommandTest.cpp
(right):

https://codereview.chromium.org/2121613003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommandTest.cpp:27:
TEST(ReplaceSelectionCommandTest, styleTagsInPastedHeadIncludedInContent)
BTW, you can use EditingTestBase.
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/editing/E...

[tkent, 2016-07-06 04:06:44]

https://codereview.chromium.org/2121613003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/ParserContentPolicy.h (left):

https://codereview.chromium.org/2121613003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/ParserContentPolicy.h:43: static inline
ParserContentPolicy disallowScriptingContent(ParserContentPolicy
parserContentPolicy)
Is this change related to the purpose of this CL?

[yosin_UTC9, 2016-07-06 05:52:35]

On 2016/07/06 at 04:06:44, tkent wrote:
>
https://codereview.chromium.org/2121613003/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/dom/ParserContentPolicy.h (left):
> 
>
https://codereview.chromium.org/2121613003/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/dom/ParserContentPolicy.h:43: static inline
ParserContentPolicy disallowScriptingContent(ParserContentPolicy
parserContentPolicy)
> Is this change related to the purpose of this CL?

I would like to add test to ReplaceSelectionCommandTest.cpp,
http://crrev.com/2122003003
So, could you use EditingTestBase?
Thanks in advance!

[dominicc (has gone to gerrit), 2016-07-06 06:52:14]

PTAL

https://codereview.chromium.org/2121613003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/ParserContentPolicy.h (left):

https://codereview.chromium.org/2121613003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/ParserContentPolicy.h:43: static inline
ParserContentPolicy disallowScriptingContent(ParserContentPolicy
parserContentPolicy)
On 2016/07/06 at 04:06:44, tkent wrote:
> Is this change related to the purpose of this CL?

No, just some opportunistic clean up--this is unused. I can submit a separate
patch if you like.

[yosin_UTC9, 2016-07-06 06:55:17]

On 2016/07/06 at 06:52:14, dominicc wrote:
> PTAL
> 
>
https://codereview.chromium.org/2121613003/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/dom/ParserContentPolicy.h (left):
> 
>
https://codereview.chromium.org/2121613003/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/dom/ParserContentPolicy.h:43: static inline
ParserContentPolicy disallowScriptingContent(ParserContentPolicy
parserContentPolicy)
> On 2016/07/06 at 04:06:44, tkent wrote:
> > Is this change related to the purpose of this CL?
> 
> No, just some opportunistic clean up--this is unused. I can submit a separate
patch if you like.

Yes, please as usual Blink commits. ;-)

[dominicc (has gone to gerrit), 2016-07-06 07:25:50]

The CQ bit was checked by dominicc@chromium.org

[dominicc (has gone to gerrit), 2016-07-06 07:25:50]

The patchset sent to the CQ was uploaded after l-g-t-m from yosin@chromium.org
Link to the patchset: https://codereview.chromium.org/2121613003/#ps60001
(title: "EditorTestBase.")

[commit-bot: I haz the power, 2016-07-06 07:26:00]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-06 08:28:16]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-07-06 08:28:30]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-06 08:30:20]

Description was changed from

==========
Include style tags when pasting HTML content.

Applications such as Excel put HTML content on the clipboard which
includes STYLE elements. Users expect the styles to remain when
pasting this content.

Chromium used to strip STYLE elements like WebKit. This change leaves
the STYLE elements in place like Firefox and Edge. This means pasted
content could change the formatting of the surrounding page, but
editors could work around this by processing STYLE tags.

BUG=121163
==========

to

==========
Include style tags when pasting HTML content.

Applications such as Excel put HTML content on the clipboard which
includes STYLE elements. Users expect the styles to remain when
pasting this content.

Chromium used to strip STYLE elements like WebKit. This change leaves
the STYLE elements in place like Firefox and Edge. This means pasted
content could change the formatting of the surrounding page, but
editors could work around this by processing STYLE tags.

BUG=121163

Committed: https://crrev.com/8ebfae3567b1bfe350286345bb0c338b9f354fe1
Cr-Commit-Position: refs/heads/master@{#403867}
==========

[commit-bot: I haz the power, 2016-07-06 08:30:21]

Patchset 4 (id:??) landed as
https://crrev.com/8ebfae3567b1bfe350286345bb0c338b9f354fe1
Cr-Commit-Position: refs/heads/master@{#403867}