Keep formatting tags included when it is cut or copied.

Keep formatting tags included when it is cut or copied.

When we copy/cut a formatting tag without the highest node.
the formatting tag's text can be wrapped by <span> tag
instead of the formatting tag. This CL allows to keep
formatting tags included when it is cut or copied.

BUG=634482
TEST=editing/pasteboard/cut-paste-formatting-tag.html

Committed: https://crrev.com/f77d0eaedb2787483c147b973bcf7e7de82bf0f3
Cr-Commit-Position: refs/heads/master@{#411883}

[joone, 2016-08-10 00:25:07]

The CQ bit was checked by joone.hur@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-10 00:25:38]

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

[joone, 2016-08-10 00:33:44]

The CQ bit was checked by joone.hur@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-10 00:33:53]

Dry run: 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-08-10 02:02:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-10 02:02:57]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[joone, 2016-08-10 18:21:55]

The CQ bit was checked by joone.hur@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-10 18:22:31]

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

[joone, 2016-08-10 18:24:06]

Patchset #3 (id:40001) has been deleted

[commit-bot: I haz the power, 2016-08-10 18:26:09]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[joone, 2016-08-10 18:28:34]

The CQ bit was checked by joone.hur@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-10 18:29:12]

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

[joone, 2016-08-10 20:19:16]

Description was changed from

==========
Keep formatting tags included when it is copied and pasted.

When we copy/cut a formatting tags without the highest node, the
formatting tag can be transformed to a span tag with inline-styles.

This CL keeps formatting tags included when it is copied and pasted.

BUG=634482
TEST=editing/pasteboard/cut-paste-formatting-tag.html
==========

to

==========
Keep formatting tags included when it is copied or cut.

When we copy/cut a formatting tags without the highest node, the
formatting tag can be transformed to a span tag with inline-styles.

This CL keeps formatting tags included when it is copied and pasted.

BUG=634482
TEST=editing/pasteboard/cut-paste-formatting-tag.html
==========

[joone, 2016-08-10 20:21:08]

Description was changed from

==========
Keep formatting tags included when it is copied or cut.

When we copy/cut a formatting tags without the highest node, the
formatting tag can be transformed to a span tag with inline-styles.

This CL keeps formatting tags included when it is copied and pasted.

BUG=634482
TEST=editing/pasteboard/cut-paste-formatting-tag.html
==========

to

==========
Keep formatting tags included when it is cut or copied.

When we copy/cut a formatting tags without the highest node, the
formatting tag can be transformed to a span tag with inline-styles.

This CL keeps formatting tags included when it is copied and pasted.

BUG=634482
TEST=editing/pasteboard/cut-paste-formatting-tag.html
==========

[joone, 2016-08-10 20:21:27]

Description was changed from

==========
Keep formatting tags included when it is cut or copied.

When we copy/cut a formatting tags without the highest node, the
formatting tag can be transformed to a span tag with inline-styles.

This CL keeps formatting tags included when it is copied and pasted.

BUG=634482
TEST=editing/pasteboard/cut-paste-formatting-tag.html
==========

to

==========
Keep formatting tags included when it is cut or copied.

When we copy/cut a formatting tag without the highest node, the
formatting tag can be transformed to a span tag with inline-styles.

This CL keeps formatting tags included when it is copied and pasted.

BUG=634482
TEST=editing/pasteboard/cut-paste-formatting-tag.html
==========

[joone, 2016-08-10 20:22:17]

Description was changed from

==========
Keep formatting tags included when it is cut or copied.

When we copy/cut a formatting tag without the highest node, the
formatting tag can be transformed to a span tag with inline-styles.

This CL keeps formatting tags included when it is copied and pasted.

BUG=634482
TEST=editing/pasteboard/cut-paste-formatting-tag.html
==========

to

==========
Keep formatting tags included when it is cut or copied.

When we copy/cut a formatting tag without the highest node, the
formatting tag can be transformed to a span tag with inline-styles.

This CL keeps formatting tags included when it is cut or copied.


BUG=634482
TEST=editing/pasteboard/cut-paste-formatting-tag.html
==========

[joone, 2016-08-10 20:30:01]

Description was changed from

==========
Keep formatting tags included when it is cut or copied.

When we copy/cut a formatting tag without the highest node, the
formatting tag can be transformed to a span tag with inline-styles.

This CL keeps formatting tags included when it is cut or copied.


BUG=634482
TEST=editing/pasteboard/cut-paste-formatting-tag.html
==========

to

==========
Keep formatting tags included when it is cut or copied.

When we copy/cut a formatting tag without the highest node.
the formatting tag's text can be wrapped by <span> tag
instead of the formatting tag. This CL allows to keep
formatting tags included when it is cut or copied.

BUG=634482
TEST=editing/pasteboard/cut-paste-formatting-tag.html
==========

[joone, 2016-08-10 21:31:31]

joone.hur@intel.com changed reviewers:
+ yosin@chromium.org

[joone, 2016-08-10 21:31:32]

Hi yosin@ could you review this CL?

https://codereview.chromium.org/2229703004/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/editing/pasteboard/cut-paste-formatting-tag.html
(right):

https://codereview.chromium.org/2229703004/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/editing/pasteboard/cut-paste-formatting-tag.html:15:
'<div contenteditable id="editor" style="width: 300px; height: 250px; border:
1px solid black"><br><div><b>foo|</b></div></div>'),
The layout test result is a bit different from running
https://bugs.chromium.org/p/chromium/issues/attachmentText?aid=245594 in
Chromium browser:

This is the result of Chrome browser without this fix:
<div contenteditable="true" id="ce" style="width: 300px; height: 250px; border:
1px solid black">
  <br class="Apple-interchange-newline">
  <span style="font-weight: bold;">aaaa</span>
  <br>
</div>

This is the layout test result without this fix:
<div contenteditable id="editor" style="width: 300px; height: 250px; border: 1px
solid black">
  <br>
  <div>
    <span style="font-weight: bold;">foo|</span>
  </div>
</div>


This is the result of Chrome browser with this fix:
<div contenteditable="true" id="ce" style="width: 300px; height: 250px; border:
1px solid black">
  <b><br class="Apple-interchange-newline">aaaa</b>
  <br>
</div>


This is the layout test result with this fix:
<div contenteditable id="editor" style="width: 300px; height: 250px; border: 1px
solid black">
  <br>
  <div>
    <b>foo|</b>
  </div>
</div>

[commit-bot: I haz the power, 2016-08-10 21:35:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-10 21:35:21]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[yosin_UTC9, 2016-08-12 01:37:35]

lgtm w/ small stylish nits

https://codereview.chromium.org/2229703004/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/editing/pasteboard/cut-paste-formatting-tag.html
(right):

https://codereview.chromium.org/2229703004/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/editing/pasteboard/cut-paste-formatting-tag.html:10:
var editor = selection.document.getElementById('editor');
nit: Could you get rid of |var editor = ...| since this script doesn't use
|editor|?

https://codereview.chromium.org/2229703004/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/EditingUtilities.cpp (right):

https://codereview.chromium.org/2229703004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingUtilities.cpp:1139: bool
isPresentationalHTMLElement(const Node* node)
Could you make |isPresentationalHTMLElement() to take |const Node&|,
since this function doesn't take |nullptr|?

[joone, 2016-08-12 22:02:07]

The CQ bit was checked by joone.hur@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-12 22:03:08]

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

[joone, 2016-08-12 22:12:21]

Thanks for review!

https://codereview.chromium.org/2229703004/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/editing/pasteboard/cut-paste-formatting-tag.html
(right):

https://codereview.chromium.org/2229703004/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/editing/pasteboard/cut-paste-formatting-tag.html:10:
var editor = selection.document.getElementById('editor');
On 2016/08/12 01:37:34, Yosi_UTC9 wrote:
> nit: Could you get rid of |var editor = ...| since this script doesn't use
> |editor|?

Done.

https://codereview.chromium.org/2229703004/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/EditingUtilities.cpp (right):

https://codereview.chromium.org/2229703004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingUtilities.cpp:1139: bool
isPresentationalHTMLElement(const Node* node)
On 2016/08/12 01:37:35, Yosi_UTC9 wrote:
> Could you make |isPresentationalHTMLElement() to take |const Node&|,
> since this function doesn't take |nullptr|?

Maybe no, because we need to change other methods because they are used as
parameter of highestEnclosingNodeOfType().
=> isPresentationalHTMLElement, isMailHTMLBlockquoteElement, isInline,
isHTMLHeaderElement, isInlineHTMLElementWithStyle.
Other methods may take |nullptr| so we need to figure out if those methods
really take |nullptr|.

[joone, 2016-08-12 22:13:45]

On 2016/08/12 22:12:21, joone wrote:

> On 2016/08/12 01:37:35, Yosi_UTC9 wrote:
> > Could you make |isPresentationalHTMLElement() to take |const Node&|,
> > since this function doesn't take |nullptr|?
> 
> Maybe no, because we need to change other methods because they are used as
> parameter of highestEnclosingNodeOfType().
> => isPresentationalHTMLElement, isMailHTMLBlockquoteElement, isInline,
> isHTMLHeaderElement, isInlineHTMLElementWithStyle.
> Other methods may take |nullptr| so we need to figure out if those methods
> really take |nullptr|.

If it is possible, I will prepare for another CL.

[commit-bot: I haz the power, 2016-08-13 01:37:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-13 01:37:07]

Dry run: This issue passed the CQ dry run.

[joone, 2016-08-13 06:44:00]

The CQ bit was checked by joone.hur@intel.com

[joone, 2016-08-13 06:44:00]

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/2229703004/#ps80001
(title: "remove editor variable in test case")

[commit-bot: I haz the power, 2016-08-13 06:44:09]

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-08-13 06:47:24]

Description was changed from

==========
Keep formatting tags included when it is cut or copied.

When we copy/cut a formatting tag without the highest node.
the formatting tag's text can be wrapped by <span> tag
instead of the formatting tag. This CL allows to keep
formatting tags included when it is cut or copied.

BUG=634482
TEST=editing/pasteboard/cut-paste-formatting-tag.html
==========

to

==========
Keep formatting tags included when it is cut or copied.

When we copy/cut a formatting tag without the highest node.
the formatting tag's text can be wrapped by <span> tag
instead of the formatting tag. This CL allows to keep
formatting tags included when it is cut or copied.

BUG=634482
TEST=editing/pasteboard/cut-paste-formatting-tag.html
==========

[commit-bot: I haz the power, 2016-08-13 06:47:25]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-08-13 06:49:00]

Description was changed from

==========
Keep formatting tags included when it is cut or copied.

When we copy/cut a formatting tag without the highest node.
the formatting tag's text can be wrapped by <span> tag
instead of the formatting tag. This CL allows to keep
formatting tags included when it is cut or copied.

BUG=634482
TEST=editing/pasteboard/cut-paste-formatting-tag.html
==========

to

==========
Keep formatting tags included when it is cut or copied.

When we copy/cut a formatting tag without the highest node.
the formatting tag's text can be wrapped by <span> tag
instead of the formatting tag. This CL allows to keep
formatting tags included when it is cut or copied.

BUG=634482
TEST=editing/pasteboard/cut-paste-formatting-tag.html

Committed: https://crrev.com/f77d0eaedb2787483c147b973bcf7e7de82bf0f3
Cr-Commit-Position: refs/heads/master@{#411883}
==========

[commit-bot: I haz the power, 2016-08-13 06:49:01]

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