Remove style spans to follow the styles of the block element

Remove style spans to follow the styles of the block element

This CL removes style spans to follow the styles of the block
element(li, pre, td, and h1~6) when the text of the pasted
or merged element becomes a part of the block element.

BUG=226941
TEST=third_party/WebKit/LayoutTests/editing/deleting/backspace-merge-into-block-element.html

Committed: https://crrev.com/5220699381aae450f22d1ea3805141deb139f8f3
Cr-Commit-Position: refs/heads/master@{#402659}

[joone, 2016-06-28 00:44:35]

Hi yosin@, This CL deals with the similar cases to this fix:
https://codereview.chromium.org/2072093002/

[joone, 2016-06-28 00:45:59]

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

[joone, 2016-06-28 00:45:59]

Could you review this CL?

[yosin_UTC9, 2016-06-28 01:27:50]

https://codereview.chromium.org/2102913002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/editing/deleting/backspace-merge-into-block.html
(right):

https://codereview.chromium.org/2102913002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/editing/deleting/backspace-merge-into-block.html:33:
range.setStart(editor.childNodes[2], 0);
Better to use "id" attribute to be more specific.
I sometimes forget to count "\n" and whitespaces as child node.

https://codereview.chromium.org/2102913002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/editing/deleting/backspace-merge-into-block.html:36:
selection.addRange(range);
|selection.collapse(editor.childNodes[2], 0)| is shorter.

https://codereview.chromium.org/2102913002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/editing/deleting/backspace-merge-into-block.html:41:
assert_equals(html, '<h1>Heading 1:paragraph was merged.</h1>');
Can we use |assert_selection()|?
See http://crrev.com/396768 as a sample.
It seems my brain loses |assert_selection()|. I should suggest it for last CL
(crrev.com/2064473002). (T_T)

https://codereview.chromium.org/2102913002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp
(right):

https://codereview.chromium.org/2102913002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp:803:
return element.hasTagName(liTag)
Is it better to check "display:block", "display:table-cell",
"display:list-item", etc, rather than tag name?
Since, Web author can override meaning of tag name, e.g. "<div
style="display:inline">...</div>"

https://codereview.chromium.org/2102913002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp:810:
|| element.hasTagName(h5Tag);
Could you explain why do you omit H6?
https://developer.mozilla.org/de/docs/Web/HTML/Element/h1-h6

https://codereview.chromium.org/2102913002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp:833:
if (node->layoutObject() && node->layoutObject()->isInline()) {
Let's use Node::computedStyle() rather than layout object to reduce dependency
of layout tree.

[joone, 2016-06-28 08:25:17]

Description was changed from

==========
Remove style spans to follow the styles of the block element

This CL removes style spans to follow the styles of the block
element(li, pre, td, and h1~5) when the text of the pasted
or merged element becomes a part of the block element.

BUG=226941
TEST=third_party/WebKit/LayoutTests/editing/deleting/backspace-merge-into-block-element.html
==========

to

==========
Remove style spans to follow the styles of the block element

This CL removes style spans to follow the styles of the block
element(li, pre, td, and h1~6) when the text of the pasted
or merged element becomes a part of the block element.

BUG=226941
TEST=third_party/WebKit/LayoutTests/editing/deleting/backspace-merge-into-block-element.html
==========

[joone, 2016-06-28 08:41:43]

Thank you for the review. :-) The test case became simple. o/

https://codereview.chromium.org/2102913002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/editing/deleting/backspace-merge-into-block.html
(right):

https://codereview.chromium.org/2102913002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/editing/deleting/backspace-merge-into-block.html:41:
assert_equals(html, '<h1>Heading 1:paragraph was merged.</h1>');
On 2016/06/28 01:27:50, Yosi_UTC9 wrote:
> Can we use |assert_selection()|?
> See http://crrev.com/396768 as a sample.
> It seems my brain loses |assert_selection()|. I should suggest it for last CL
> (crrev.com/2064473002). (T_T)

Done.

https://codereview.chromium.org/2102913002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp
(right):

https://codereview.chromium.org/2102913002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp:803:
return element.hasTagName(liTag)
On 2016/06/28 01:27:50, Yosi_UTC9 wrote:
> Is it better to check "display:block", "display:table-cell",
> "display:list-item", etc, rather than tag name?
> Since, Web author can override meaning of tag name, e.g. "<div
> style="display:inline">...</div>"

Done.

https://codereview.chromium.org/2102913002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp:810:
|| element.hasTagName(h5Tag);
On 2016/06/28 01:27:50, Yosi_UTC9 wrote:
> Could you explain why do you omit H6?
> https://developer.mozilla.org/de/docs/Web/HTML/Element/h1-h6

I just missed h6.

https://codereview.chromium.org/2102913002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp:833:
if (node->layoutObject() && node->layoutObject()->isInline()) {
On 2016/06/28 01:27:50, Yosi_UTC9 wrote:
> Let's use Node::computedStyle() rather than layout object to reduce dependency
> of layout tree.

Done.

[yosin_UTC9, 2016-06-29 00:50:16]

The CQ bit was checked by yosin@chromium.org

[yosin_UTC9, 2016-06-29 00:50:16]

lgtm

[commit-bot: I haz the power, 2016-06-29 00:51:10]

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-06-29 02:15:16]

Description was changed from

==========
Remove style spans to follow the styles of the block element

This CL removes style spans to follow the styles of the block
element(li, pre, td, and h1~6) when the text of the pasted
or merged element becomes a part of the block element.

BUG=226941
TEST=third_party/WebKit/LayoutTests/editing/deleting/backspace-merge-into-block-element.html
==========

to

==========
Remove style spans to follow the styles of the block element

This CL removes style spans to follow the styles of the block
element(li, pre, td, and h1~6) when the text of the pasted
or merged element becomes a part of the block element.

BUG=226941
TEST=third_party/WebKit/LayoutTests/editing/deleting/backspace-merge-into-block-element.html
==========

[commit-bot: I haz the power, 2016-06-29 02:15:18]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-29 02:17:41]

Description was changed from

==========
Remove style spans to follow the styles of the block element

This CL removes style spans to follow the styles of the block
element(li, pre, td, and h1~6) when the text of the pasted
or merged element becomes a part of the block element.

BUG=226941
TEST=third_party/WebKit/LayoutTests/editing/deleting/backspace-merge-into-block-element.html
==========

to

==========
Remove style spans to follow the styles of the block element

This CL removes style spans to follow the styles of the block
element(li, pre, td, and h1~6) when the text of the pasted
or merged element becomes a part of the block element.

BUG=226941
TEST=third_party/WebKit/LayoutTests/editing/deleting/backspace-merge-into-block-element.html

Committed: https://crrev.com/5220699381aae450f22d1ea3805141deb139f8f3
Cr-Commit-Position: refs/heads/master@{#402659}
==========

[commit-bot: I haz the power, 2016-06-29 02:17:42]

Patchset 2 (id:??) landed as
https://crrev.com/5220699381aae450f22d1ea3805141deb139f8f3
Cr-Commit-Position: refs/heads/master@{#402659}

[shans, 2016-06-29 03:05:42]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/2109973002/ by shans@chromium.org.

The reason for reverting is: Assignment within conditional on line 836 of
ReplaceSelectionCommand.cpp is breaking Win8 build:

https://build.chromium.org/p/chromium.win/builders/Win8%20GYP%20%28dbg%29/bui....