New issue
Advanced search Search tips

Issue 889737 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

CompositeEditCommand::EnsureUndoStep() should use valid root editable element

Project Member Reported by yosin@chromium.org, Sep 27

Issue description

CompositeEditCommand::EnsureUndoStep() uses invalid root editable element when creating UndoStep. We should avoid it.

Following tests do this:
* editing/pasteboard/4947130.html
* editing/pasteboard/drag-drop-iframe-refresh-crash.html
* editing/pasteboard/drag-drop-list.html
* editing/pasteboard/drag-drop-url-with-style.html
* editing/pasteboard/drag-image-in-about-blank-frame.html
* editing/pasteboard/drag-list-item.html
* editing/selection/mouse/drag_table_cell.html

# Possible solution
* Make CompositeEditCommand to hold root editable element for starting and ending selection.
* Make UndoStep::Create() to take selection with root editable element

# Sample Stack Trace
PositionTemplate<blink::EditingAlgorithm<blink::NodeTraversal> >::ComputeContainerNode() Line 196
RootEditableElementOf(const blink::PositionTemplate<blink::EditingAlgorithm<blink::NodeTraversal> > & p) Line 498
UndoStep::UndoStep(blink::Document * document, const blink::SelectionForUndoStep & starting_selection, const blink::SelectionForUndoStep & ending_selection, blink::InputEvent::InputType input_type) Line 43
UndoStep::Create(blink::Document * document, const blink::SelectionForUndoStep & starting_selection, const blink::SelectionForUndoStep & ending_selection, blink::InputEvent::InputType input_type) Line 27
>	CompositeEditCommand::EnsureUndoStep() Line 173
CompositeEditCommand::AppliedEditing() Line 2047
CompositeEditCommand::Apply() Line 165
Editor::DeleteSelectionWithSmartDelete(blink::DeleteMode delete_mode, blink::InputEvent::InputType input_type, const blink::PositionTemplate<blink::EditingAlgorithm<blink::NodeTraversal> > & reference_move_position) Line 282
Editor::DeleteSelectionAfterDraggingWithEvents(blink::Element * drag_source, blink::DeleteMode delete_mode, const blink::PositionTemplate<blink::EditingAlgorithm<blink::NodeTraversal> > & reference_move_position) Line 371
DragController::ConcludeEditDrag(blink::DragData * drag_data) Line 636
DragController::PerformDrag(blink::DragData * drag_data, blink::LocalFrame & local_root) Line 284



 
Description: Show this description
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e88bfe617cb1f76446a8a0c32b5b6782bbbf9314

commit e88bfe617cb1f76446a8a0c32b5b6782bbbf9314
Author: Yoshifumi Inoue <yosin@chromium.org>
Date: Thu Sep 27 08:43:52 2018

Add DCHECK for Before/After position

This patch adds |DCHECK()| to narrow down the case of returning null from
|Position::ComputeContainerNode()| for helping to search the root cause of
the bug[1] caused by |ComputeContainerNode()| returning null for non-null
position.

This patch also changes tests using before/after position of shadow root.
Node: document and document fragment, including shadow root, can not be an
anchor node of before/after position, since these position can not be represent
by RangeBoundaryPoint == container node with offset of child.

[1] https://crbug.com/882592 SelectionEditor should not call
Node::ContainsIncludingHostElements() with null

Bug: 882592, 889737
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I9c7b0c8ad1c97d20a9080aa9baddea69faf805c5
Reviewed-on: https://chromium-review.googlesource.com/1248361
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Yoichi Osato <yoichio@chromium.org>
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594636}
[modify] https://crrev.com/e88bfe617cb1f76446a8a0c32b5b6782bbbf9314/third_party/blink/renderer/core/dom/document_test.cc
[modify] https://crrev.com/e88bfe617cb1f76446a8a0c32b5b6782bbbf9314/third_party/blink/renderer/core/editing/position.cc
[modify] https://crrev.com/e88bfe617cb1f76446a8a0c32b5b6782bbbf9314/third_party/blink/renderer/core/editing/position_test.cc

Sign in to add a comment