Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(17)

Issue 291143002: Don't indent if selection is none. (Closed)

Created:
6 years, 8 months ago by yosin_UTC9
Modified:
6 years, 8 months ago
Reviewers:
yoichio, Yuta Kitamura
CC:
blink-reviews
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Don't indent if selection is none. ApplyBlockElementCommand::doApply() assumes selection to indent points non-null position. However, there is a chance this assumption is false, e.g. HTML used in "indent-button-crash.html". This patch makes ApplyBlockElementCommand::doApply() to check whether assumption is satisfied or not. BUG=368133 TEST=LayoutTests/editing/execCommand/indent-button-crash.html Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=174671

Patch Set 1 : 2014-05-20T05:00:38 #

Total comments: 8

Patch Set 2 : 2014-05-20T06:41:02 #

Patch Set 3 : 2014-05-21T10:27:57 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+38 lines, -1 line) Patch
A LayoutTests/editing/execCommand/indent-button-crash.html View 1 2 1 chunk +33 lines, -0 lines 0 comments Download
A + LayoutTests/editing/execCommand/indent-button-crash-expected.txt View 0 chunks +-1 lines, --1 lines 0 comments Download
M Source/core/editing/ApplyBlockElementCommand.cpp View 1 2 1 chunk +6 lines, -2 lines 0 comments Download

Messages

Total messages: 9 (0 generated)
yosin_UTC9
Could you review this patch? Thanks in advance.
6 years, 8 months ago (2014-05-20 08:46:53 UTC) #1
yoichio
lgtm With a nit. https://codereview.chromium.org/291143002/diff/1/LayoutTests/editing/execCommand/indent-button-crash.html File LayoutTests/editing/execCommand/indent-button-crash.html (right): https://codereview.chromium.org/291143002/diff/1/LayoutTests/editing/execCommand/indent-button-crash.html#newcode6 LayoutTests/editing/execCommand/indent-button-crash.html:6: var aoScriptElements = document.getElementsByTagName('script'); |aoScriptElements| ...
6 years, 8 months ago (2014-05-20 09:19:50 UTC) #2
yosin_UTC9
PTAL https://codereview.chromium.org/291143002/diff/1/LayoutTests/editing/execCommand/indent-button-crash.html File LayoutTests/editing/execCommand/indent-button-crash.html (right): https://codereview.chromium.org/291143002/diff/1/LayoutTests/editing/execCommand/indent-button-crash.html#newcode6 LayoutTests/editing/execCommand/indent-button-crash.html:6: var aoScriptElements = document.getElementsByTagName('script'); On 2014/05/20 09:19:50, yoichio ...
6 years, 8 months ago (2014-05-20 09:42:17 UTC) #3
Yuta Kitamura
Please wrap the change description in every 80-ish characters. Unbounded lines in commit log looks ...
6 years, 8 months ago (2014-05-20 09:44:35 UTC) #4
yosin_UTC9
PTAL https://codereview.chromium.org/291143002/diff/1/LayoutTests/editing/execCommand/indent-button-crash.html File LayoutTests/editing/execCommand/indent-button-crash.html (right): https://codereview.chromium.org/291143002/diff/1/LayoutTests/editing/execCommand/indent-button-crash.html#newcode8 LayoutTests/editing/execCommand/indent-button-crash.html:8: aoScriptElements[i].parentNode.removeChild(aoScriptElements[i]); On 2014/05/20 09:44:36, Yuta Kitamura wrote: > ...
6 years, 8 months ago (2014-05-21 03:44:07 UTC) #5
Yuta Kitamura
LGTM, sorry for delay!
6 years, 8 months ago (2014-05-23 06:52:24 UTC) #6
yosin_UTC9
The CQ bit was checked by yosin@chromium.org
6 years, 8 months ago (2014-05-23 07:02:58 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yosin@chromium.org/291143002/60001
6 years, 8 months ago (2014-05-23 07:03:35 UTC) #8
commit-bot: I haz the power
6 years, 8 months ago (2014-05-23 10:35:26 UTC) #9
Message was sent while issue was closed.
Change committed as 174671

Powered by Google App Engine
This is Rietveld 408576698