Make replacing selection in heading/paragraph with heading/paragraph to work

Before this patch, replacing selection in heading/paragraph with heading/paragraph gets assertion at InsertNodeBeforeCommand constructor, or crash when assertion disabled. It tries to move replaced heading/paragraph out of ancestor, ReplaceSelectionCommand::moveNodeOutOfAncestor().

This patch changes ReplaceSelectionCommand::moveNodeOutOfAncestor() to avoid the assertion.

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

[yosin_UTC9, 2013-06-06 08:27:23]

Hi guys,
Could you review this patch for crash bug?
Thanks in advance.
-yosi

[yosin_UTC9, 2013-06-11 03:33:09]

ping...

[rniwa-cr, 2013-06-11 06:32:00]

https://codereview.chromium.org/16001010/diff/5001/Source/core/editing/Replac...
File Source/core/editing/ReplaceSelectionCommand.cpp (right):

https://codereview.chromium.org/16001010/diff/5001/Source/core/editing/Replac...
Source/core/editing/ReplaceSelectionCommand.cpp:641: if
(ancestor->parentNode()->attached() &&
!ancestor->parentNode()->rendererIsEditable())
Why are we checking attached()? That doesn't make much sense given
rendererIsEditable looks for the node with a renderer. If this is causing a
problem, then we might be in a need of another style resolution.

[yosin_UTC9, 2013-06-11 08:07:41]

Could you review this patch?
Thanks in advance.

https://codereview.chromium.org/16001010/diff/5001/Source/core/editing/Replac...
File Source/core/editing/ReplaceSelectionCommand.cpp (right):

https://codereview.chromium.org/16001010/diff/5001/Source/core/editing/Replac...
Source/core/editing/ReplaceSelectionCommand.cpp:641: if
(ancestor->parentNode()->attached() &&
!ancestor->parentNode()->rendererIsEditable())
On 2013/06/11 06:32:00, rniwa-cr wrote:
> Why are we checking attached()? That doesn't make much sense given
> rendererIsEditable looks for the node with a renderer. If this is causing a
> problem, then we might be in a need of another style resolution.
You're right.
Remove attached() check.

[rniwa-cr, 2013-06-13 05:22:50]

The new patch looks correct.

[eseidel, 2013-06-25 21:12:49]

lgtm

[commit-bot: I haz the power, 2013-06-25 21:14:40]

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

[commit-bot: I haz the power, 2013-06-25 22:42:00]

Change committed as 153031