Commit e4f7d9c3 authored by Oriol Brufau's avatar Oriol Brufau Committed by Commit Bot

[editing] Avoid DCHECK failure with InsertOrderedList

In InsertListCommand::UnlistifyParagraph, if `list_element` has no <li>,
then `list_child_node` will be a non-<li> child. In that case,
`previous_list_child` and `next_list_child` may end up being the same
`list_child_node` if it has multiple block children. This would make
some DCHECKs fail.

However, the behavior seems fine, so this patch changes the DCHECKs to
ensure that `previous_list_child` and `next_list_child` are obtained
from nodes different than the block where the caret is.

Bug: 1138054

TEST=InsertListCommandTest.UnlistifyParagraphCrashOnNonLi

Change-Id: I18631b1c90d59539433373a2139aab36115e59ec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2497161
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821691}
parent 2caca2af
...@@ -485,12 +485,12 @@ void InsertListCommand::UnlistifyParagraph( ...@@ -485,12 +485,12 @@ void InsertListCommand::UnlistifyParagraph(
end = EndOfParagraph(start, kCanSkipOverEditingBoundary); end = EndOfParagraph(start, kCanSkipOverEditingBoundary);
// InsertListCommandTest.UnlistifyParagraphCrashOnRemoveStyle reaches here. // InsertListCommandTest.UnlistifyParagraphCrashOnRemoveStyle reaches here.
ABORT_EDITING_COMMAND_IF(start.DeepEquivalent() == end.DeepEquivalent()); ABORT_EDITING_COMMAND_IF(start.DeepEquivalent() == end.DeepEquivalent());
next_list_child = EnclosingListChild( Node* next = NextPositionOf(end).DeepEquivalent().AnchorNode();
NextPositionOf(end).DeepEquivalent().AnchorNode(), list_element); DCHECK_NE(next, end.DeepEquivalent().AnchorNode());
DCHECK_NE(next_list_child, list_child_node); next_list_child = EnclosingListChild(next, list_element);
previous_list_child = EnclosingListChild( Node* previous = PreviousPositionOf(start).DeepEquivalent().AnchorNode();
PreviousPositionOf(start).DeepEquivalent().AnchorNode(), list_element); DCHECK_NE(previous, start.DeepEquivalent().AnchorNode());
DCHECK_NE(previous_list_child, list_child_node); previous_list_child = EnclosingListChild(previous, list_element);
} }
// Helpers for making |start| and |end| valid again after DOM changes. // Helpers for making |start| and |end| valid again after DOM changes.
......
...@@ -71,6 +71,21 @@ TEST_F(InsertListCommandTest, UnlistifyParagraphCrashOnVisuallyEmptyParagraph) { ...@@ -71,6 +71,21 @@ TEST_F(InsertListCommandTest, UnlistifyParagraphCrashOnVisuallyEmptyParagraph) {
GetSelectionTextFromBody()); GetSelectionTextFromBody());
} }
TEST_F(InsertListCommandTest, UnlistifyParagraphCrashOnNonLi) {
// Checks that InsertOrderedList does not cause a crash when the caret is in a
// non-<li> child of a list which contains non-<li> blocks.
GetDocument().setDesignMode("on");
Selection().SetSelection(SetSelectionTextToBody("<ol><div>|"
"<p>foo</p><p>bar</p>"
"</div></ol>"),
SetSelectionOptions());
auto* command = MakeGarbageCollected<InsertListCommand>(
GetDocument(), InsertListCommand::kOrderedList);
// Crash happens here.
EXPECT_TRUE(command->Apply());
EXPECT_EQ("|foo<br><ol><p>bar</p></ol>", GetSelectionTextFromBody());
}
// Refer https://crbug.com/798176 // Refer https://crbug.com/798176
TEST_F(InsertListCommandTest, CleanupNodeSameAsDestinationNode) { TEST_F(InsertListCommandTest, CleanupNodeSameAsDestinationNode) {
GetDocument().setDesignMode("on"); GetDocument().setDesignMode("on");
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment