Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(42)

Issue 14995014: Don't move uneditable style and link elements during deletion (Closed)

Created:
7 years, 7 months ago by yosin_UTC9
Modified:
7 years, 7 months ago
CC:
blink-reviews, eae+blinkwatch
Visibility:
Public.

Description

During deletion, we make style and link elements to direct child of editable root in DeleteSelectionCommand::makeStylingElementsDirectChildrenOfEditableRootToPreventStyleLoss(). Although, if style and link elements don't have root editable element, e.g. inside contentnEditable="false", appendNode() refers null pointer. This patch changes to avoid above situation. BUG=177470 R=ojan@chromium.org Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=150296

Patch Set 1 : 2013-05-13T16:49 #

Total comments: 4

Patch Set 2 : 2013-05-14T12:59 #

Total comments: 2

Patch Set 3 : 2013-05-14T15:16 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+32 lines, -2 lines) Patch
A LayoutTests/editing/deleting/delete-uneditable-style.html View 1 2 1 chunk +22 lines, -0 lines 0 comments Download
A LayoutTests/editing/deleting/delete-uneditable-style-expected.txt View 1 1 chunk +6 lines, -0 lines 0 comments Download
M Source/core/editing/DeleteSelectionCommand.cpp View 1 chunk +4 lines, -2 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
yosin_UTC9
Could you review this patch? Thanks in advance.
7 years, 7 months ago (2013-05-13 07:56:18 UTC) #1
ojan
The C++ side looks good to me. Just a few comments about the test. It ...
7 years, 7 months ago (2013-05-14 02:40:46 UTC) #2
yosin_UTC9
PTAL. Thanks for comments. Updates a test script. Thanks in advance! https://codereview.chromium.org/14995014/diff/2001/LayoutTests/editing/deleting/delete-uneditable-style.html File LayoutTests/editing/deleting/delete-uneditable-style.html (right): ...
7 years, 7 months ago (2013-05-14 04:20:31 UTC) #3
ojan
lgtm https://codereview.chromium.org/14995014/diff/8001/LayoutTests/editing/deleting/delete-uneditable-style.html File LayoutTests/editing/deleting/delete-uneditable-style.html (right): https://codereview.chromium.org/14995014/diff/8001/LayoutTests/editing/deleting/delete-uneditable-style.html#newcode6 LayoutTests/editing/deleting/delete-uneditable-style.html:6: window.addEventListener('load', function() { This is fine. Another way ...
7 years, 7 months ago (2013-05-14 05:27:22 UTC) #4
yosin_UTC9
Thanks! I updated test script as suggested. https://chromiumcodereview.appspot.com/14995014/diff/8001/LayoutTests/editing/deleting/delete-uneditable-style.html File LayoutTests/editing/deleting/delete-uneditable-style.html (right): https://chromiumcodereview.appspot.com/14995014/diff/8001/LayoutTests/editing/deleting/delete-uneditable-style.html#newcode6 LayoutTests/editing/deleting/delete-uneditable-style.html:6: window.addEventListener('load', function() ...
7 years, 7 months ago (2013-05-14 06:17:08 UTC) #5
yosin_UTC9
7 years, 7 months ago (2013-05-14 06:17:56 UTC) #6
Message was sent while issue was closed.
Committed patchset #3 manually as r150296 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698