Don't move uneditable paragraph contents in CompositeEditCommand::moveParagraphContentsToNewBlockIfN

Don't move uneditable paragraph contents in CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessary

This patch makes CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessary
not to move uneditable paragraph contents.

In attached test case, Blink tried to move contents in uneditable ABBR element.

BUG=381076, 383811
TEST=LayoutTests/editing/execCommand/justy-center-with-uneditable-crash.html

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=176172

[yosin_UTC9, 2014-06-13 02:28:27]

Could you review this patch?
Thanks in advance.

[Yuta Kitamura, 2014-06-13 07:29:02]

https://codereview.chromium.org/335773002/diff/1/Source/core/editing/Composit...
File Source/core/editing/CompositeEditCommand.cpp (right):

https://codereview.chromium.org/335773002/diff/1/Source/core/editing/Composit...
Source/core/editing/CompositeEditCommand.cpp:905: if (!isEditablePosition(pos,
ContentIsEditable, DoNotUpdateStyle))
I'm not 100% certain whether this check should live here.

This check may perhaps be caller's responsibility, i.e.
each command should check this condition before calling
moveParagraphContentsToNewBlockIfNecessary(), and this
function just ASSERTs that this precondition is met.

WDYT?

[yosin_UTC9, 2014-06-13 09:01:27]

PTAL

https://codereview.chromium.org/335773002/diff/1/Source/core/editing/Composit...
File Source/core/editing/CompositeEditCommand.cpp (right):

https://codereview.chromium.org/335773002/diff/1/Source/core/editing/Composit...
Source/core/editing/CompositeEditCommand.cpp:905: if (!isEditablePosition(pos,
ContentIsEditable, DoNotUpdateStyle))
On 2014/06/13 07:29:01, Yuta Kitamura wrote:
> I'm not 100% certain whether this check should live here.
> 
> This check may perhaps be caller's responsibility, i.e.
> each command should check this condition before calling
> moveParagraphContentsToNewBlockIfNecessary(), and this
> function just ASSERTs that this precondition is met.
> 
> WDYT?
Agree. It should be caller's responsibility.

[yosin_UTC9, 2014-06-16 03:29:32]

ping

[Yuta Kitamura, 2014-06-16 03:33:27]

LGTM

https://codereview.chromium.org/335773002/diff/20001/LayoutTests/editing/exec...
File LayoutTests/editing/execCommand/justy-center-with-uneditable-crash.html
(right):

https://codereview.chromium.org/335773002/diff/20001/LayoutTests/editing/exec...
LayoutTests/editing/execCommand/justy-center-with-uneditable-crash.html:9:
document.execCommand("selectAll");
nit: Single quotes for string literals?

[yosin_UTC9, 2014-06-16 03:45:54]

Thanks for reviewing!
Committing...

https://codereview.chromium.org/335773002/diff/20001/LayoutTests/editing/exec...
File LayoutTests/editing/execCommand/justy-center-with-uneditable-crash.html
(right):

https://codereview.chromium.org/335773002/diff/20001/LayoutTests/editing/exec...
LayoutTests/editing/execCommand/justy-center-with-uneditable-crash.html:9:
document.execCommand("selectAll");
On 2014/06/16 03:33:26, Yuta Kitamura wrote:
> nit: Single quotes for string literals?

Done.

[yosin_UTC9, 2014-06-16 03:45:57]

The CQ bit was checked by yosin@chromium.org

[commit-bot: I haz the power, 2014-06-16 03:46:34]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/yosin@chromium.org/335773002/40001

[commit-bot: I haz the power, 2014-06-16 04:53:45]

Change committed as 176172