Issue 1690513002: Editing: Pass failure information of RemoveNodeCommand to ReplaceSelectionCommand.

Issue 1690513002: Editing: Pass failure information of RemoveNodeCommand to ReplaceSelectionCommand. (Closed)

Created:
4 years, 10 months ago by tkent

Modified:
4 years, 10 months ago

Reviewers:
yosin_UTC9

CC:
blink-reviews, chromium-reviews

Base URL:
https://chromium.googlesource.com/chromium/src.git@master

Target Ref:
refs/pending/heads/master

Project:
chromium

Visibility:
Public.

More Reviews

Description

Editing: Pass failure information of RemoveNodeCommand to ReplaceSelectionCommand. Passing EditingState through the following path in order to abort the command gracefully: - RemoveNodeCommand::doApply() - CompositeEditCommand::applyCommandToComposite() - CompositeEditCommand::removeNode() - DeleteSelectionCommand::removeNode() - DeleteSelectionCommand::handleGeneralDelete() - DeleteSelectionCommand::doApply() - CompositeEditCommand::applyCommandToComposite() - CompositeEditCommand::deleteSelection() - ReplaceSelectionCommand::doApply() This is necessary in the following case. It removes <body> from the document during InsertHTML command. <iframe src="javascript:onunload = function() { if (!top.container) { top.container = document.createElement('div'); top.container.appendChild(frameElement.parentNode); } }"> </iframe> <iframe onload=" document.counter = 1; if (document.counter <= 16) { document.designMode = 'on'; document.execCommand('selectall'); document.execCommand('inserthtml', false); }"> BUG=541002, 529929 Committed: https://crrev.com/2185123a479eddd9e20a97b89b23619e79a7af09 Cr-Commit-Position: refs/heads/master@{#374622}

Patch Set 1 #

Total comments: 5

Patch Set 2 : add a comment #

Created: 4 years, 10 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+45 lines, -23 lines)			Patch
M	third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.h	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.cpp	View	1	1 chunk	+3 lines, -2 lines	0 comments	Download
M	third_party/WebKit/Source/core/editing/commands/DeleteSelectionCommand.h	View		1 chunk	+2 lines, -2 lines	0 comments	Download
M	third_party/WebKit/Source/core/editing/commands/DeleteSelectionCommand.cpp	View	1	9 chunks	+24 lines, -11 lines	0 comments	Download
M	third_party/WebKit/Source/core/editing/commands/RemoveNodeCommand.cpp	View	1	3 chunks	+7 lines, -1 line	0 comments	Download
M	third_party/WebKit/Source/core/editing/commands/RemoveNodePreservingChildrenCommand.cpp	View		1 chunk	+2 lines, -2 lines	0 comments	Download
M	third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp	View		2 chunks	+4 lines, -2 lines	0 comments	Download
M	third_party/WebKit/Source/core/editing/commands/SimplifyMarkupCommand.cpp	View		1 chunk	+2 lines, -2 lines	0 comments	Download

Messages

Total messages: 16 (8 generated)

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages

tkent

yosin@, would you review this please? https://codereview.chromium.org/1690513002/diff/1/third_party/WebKit/Source/core/editing/commands/RemoveNodeCommand.cpp File third_party/WebKit/Source/core/editing/commands/RemoveNodeCommand.cpp (right): https://codereview.chromium.org/1690513002/diff/1/third_party/WebKit/Source/core/editing/commands/RemoveNodeCommand.cpp#newcode55 third_party/WebKit/Source/core/editing/commands/RemoveNodeCommand.cpp:55: m_node->remove(IGNORE_EXCEPTION); Here is ...

4 years, 10 months ago (2016-02-10 05:24:54 UTC) #2

yosin_UTC9

lgtm https://codereview.chromium.org/1690513002/diff/1/third_party/WebKit/Source/core/editing/commands/RemoveNodeCommand.cpp File third_party/WebKit/Source/core/editing/commands/RemoveNodeCommand.cpp (right): https://codereview.chromium.org/1690513002/diff/1/third_party/WebKit/Source/core/editing/commands/RemoveNodeCommand.cpp#newcode55 third_party/WebKit/Source/core/editing/commands/RemoveNodeCommand.cpp:55: m_node->remove(IGNORE_EXCEPTION); It it better we check an exception ...

4 years, 10 months ago (2016-02-10 05:32:17 UTC) #3

tkent

https://codereview.chromium.org/1690513002/diff/1/third_party/WebKit/Source/core/editing/commands/RemoveNodeCommand.cpp File third_party/WebKit/Source/core/editing/commands/RemoveNodeCommand.cpp (right): https://codereview.chromium.org/1690513002/diff/1/third_party/WebKit/Source/core/editing/commands/RemoveNodeCommand.cpp#newcode55 third_party/WebKit/Source/core/editing/commands/RemoveNodeCommand.cpp:55: m_node->remove(IGNORE_EXCEPTION); On 2016/02/10 at 05:32:17, Yosi_UTC9 wrote: > It ...

4 years, 10 months ago (2016-02-10 05:47:30 UTC) #5

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1690513002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1690513002/20001

4 years, 10 months ago (2016-02-10 05:47:56 UTC) #7

tkent

Description was changed from ========== Editing: Pass failure information of RemoveNodeCommand to ReplaceSelectionCommand. Passing EditingState ...

4 years, 10 months ago (2016-02-10 06:03:46 UTC) #8

Description was changed from

==========
Editing: Pass failure information of RemoveNodeCommand to
ReplaceSelectionCommand.

Passing EditingState through the following path in order to abort the command
gracefully:
 - RemoveNodeCommand::doApply()
 - CompositeEditCommand::applyCommandToComposite()
 - CompositeEditCommand::removeNode()
 - DeleteSelectionCommand::removeNode()
 - DeleteSelectionCommand::handleGeneralDelete()
 - DeleteSelectionCommand::doApply()
 - CompositeEditCommand::applyCommandToComposite()
 - CompositeEditCommand::deleteSelection()
 - ReplaceSelectionCommand::doApply()

This is necessary in the following case.  It removes <body> from the document
during InsertHTML command.

<iframe src="javascript:onunload = function() {
  if (!top.container) {
    top.container = document.createElement('div');
    top.container.appendChild(frameElement.parentNode);
  }
}">
</iframe>
<iframe onload="
  document.counter = 1;
  if (document.counter &lt;= 16) {
    document.designMode = 'on';
    document.execCommand('selectall');
    document.execCommand('inserthtml', false);
}">

BUG=541002
==========

to

==========
Editing: Pass failure information of RemoveNodeCommand to
ReplaceSelectionCommand.

Passing EditingState through the following path in order to abort the command
gracefully:
 - RemoveNodeCommand::doApply()
 - CompositeEditCommand::applyCommandToComposite()
 - CompositeEditCommand::removeNode()
 - DeleteSelectionCommand::removeNode()
 - DeleteSelectionCommand::handleGeneralDelete()
 - DeleteSelectionCommand::doApply()
 - CompositeEditCommand::applyCommandToComposite()
 - CompositeEditCommand::deleteSelection()
 - ReplaceSelectionCommand::doApply()

This is necessary in the following case.  It removes <body> from the document
during InsertHTML command.

<iframe src="javascript:onunload = function() {
  if (!top.container) {
    top.container = document.createElement('div');
    top.container.appendChild(frameElement.parentNode);
  }
}">
</iframe>
<iframe onload="
  document.counter = 1;
  if (document.counter &lt;= 16) {
    document.designMode = 'on';
    document.execCommand('selectall');
    document.execCommand('inserthtml', false);
}">

BUG=541002, 529929
==========

commit-bot: I haz the power

Failed to request the patch to try. Please note that binary files are still unsupported ...

4 years, 10 months ago (2016-02-10 07:42:49 UTC) #10

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1690513002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1690513002/20001

4 years, 10 months ago (2016-02-10 07:47:24 UTC) #13

commit-bot: I haz the power

Description was changed from ========== Editing: Pass failure information of RemoveNodeCommand to ReplaceSelectionCommand. Passing EditingState ...

4 years, 10 months ago (2016-02-10 07:52:26 UTC) #15

Message was sent while issue was closed.

Description was changed from

==========
Editing: Pass failure information of RemoveNodeCommand to
ReplaceSelectionCommand.

Passing EditingState through the following path in order to abort the command
gracefully:
 - RemoveNodeCommand::doApply()
 - CompositeEditCommand::applyCommandToComposite()
 - CompositeEditCommand::removeNode()
 - DeleteSelectionCommand::removeNode()
 - DeleteSelectionCommand::handleGeneralDelete()
 - DeleteSelectionCommand::doApply()
 - CompositeEditCommand::applyCommandToComposite()
 - CompositeEditCommand::deleteSelection()
 - ReplaceSelectionCommand::doApply()

This is necessary in the following case.  It removes <body> from the document
during InsertHTML command.

<iframe src="javascript:onunload = function() {
  if (!top.container) {
    top.container = document.createElement('div');
    top.container.appendChild(frameElement.parentNode);
  }
}">
</iframe>
<iframe onload="
  document.counter = 1;
  if (document.counter &lt;= 16) {
    document.designMode = 'on';
    document.execCommand('selectall');
    document.execCommand('inserthtml', false);
}">

BUG=541002, 529929
==========

to

==========
Editing: Pass failure information of RemoveNodeCommand to
ReplaceSelectionCommand.

Passing EditingState through the following path in order to abort the command
gracefully:
 - RemoveNodeCommand::doApply()
 - CompositeEditCommand::applyCommandToComposite()
 - CompositeEditCommand::removeNode()
 - DeleteSelectionCommand::removeNode()
 - DeleteSelectionCommand::handleGeneralDelete()
 - DeleteSelectionCommand::doApply()
 - CompositeEditCommand::applyCommandToComposite()
 - CompositeEditCommand::deleteSelection()
 - ReplaceSelectionCommand::doApply()

This is necessary in the following case.  It removes <body> from the document
during InsertHTML command.

<iframe src="javascript:onunload = function() {
  if (!top.container) {
    top.container = document.createElement('div');
    top.container.appendChild(frameElement.parentNode);
  }
}">
</iframe>
<iframe onload="
  document.counter = 1;
  if (document.counter &lt;= 16) {
    document.designMode = 'on';
    document.execCommand('selectall');
    document.execCommand('inserthtml', false);
}">

BUG=541002, 529929

Committed: https://crrev.com/2185123a479eddd9e20a97b89b23619e79a7af09
Cr-Commit-Position: refs/heads/master@{#374622}
==========

commit-bot: I haz the power

4 years, 10 months ago (2016-02-10 07:52:27 UTC) #16

Message was sent while issue was closed.

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

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages