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

Issue 67063005: Fixing indent/outdent for <li> elements containing children spanning across (Closed)

Created:
7 years, 1 month ago by arpitab_
Modified:
7 years ago
CC:
blink-reviews
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Fixing indent/outdent for <li> elements containing children spanning across multiple lines (paragraphs). For such cases the selection only encompasses children contained within the first paragraph (within the li), thereby causing incorrect behavior when indenting. Thus we need to ensure while inserting a new li element and cloning the original, that all the children are considered. With this change, an existing test: indent-pre-list.html fails because of the previous incorrect behavior. Two <li>s were created for the indented <li> with the text contained within the first paragraph forming the first and the remaining content moving to the second <li> element. This has now been fixed resulting in only a single indented <li>. Bug: http://crbug.com/321567 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=162651

Patch Set 1 #

Total comments: 5

Patch Set 2 : Modifications as per review comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+117 lines, -5 lines) Patch
A LayoutTests/editing/execCommand/indent-list-item-with-children.html View 1 1 chunk +36 lines, -0 lines 0 comments Download
A LayoutTests/editing/execCommand/indent-list-item-with-children-expected.txt View 1 chunk +70 lines, -0 lines 0 comments Download
M LayoutTests/editing/execCommand/indent-pre-list-expected.txt View 1 chunk +2 lines, -1 line 0 comments Download
M Source/core/editing/IndentOutdentCommand.cpp View 1 1 chunk +9 lines, -4 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
arpitab_
7 years, 1 month ago (2013-11-19 15:02:21 UTC) #1
yosin_UTC9
ACK https://codereview.chromium.org/67063005/diff/1/LayoutTests/editing/execCommand/indent-list-item-with-children.html File LayoutTests/editing/execCommand/indent-list-item-with-children.html (right): https://codereview.chromium.org/67063005/diff/1/LayoutTests/editing/execCommand/indent-list-item-with-children.html#newcode1 LayoutTests/editing/execCommand/indent-list-item-with-children.html:1: <html> nit: Please add <!DOCTYPE html> It seems ...
7 years, 1 month ago (2013-11-20 05:01:48 UTC) #2
arpitab_
Hi Yoshi, Thanks for the review. I'll upload another patch incorporating your review comments. https://codereview.chromium.org/67063005/diff/1/LayoutTests/editing/execCommand/indent-list-item-with-children.html ...
7 years, 1 month ago (2013-11-20 13:07:51 UTC) #3
yosin_UTC9
ACK Please wait OWNERS review for committing...
7 years, 1 month ago (2013-11-21 01:36:01 UTC) #4
ojan
lgtm
7 years ago (2013-11-26 00:11:48 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/a.bah@samsung.com/67063005/80001
7 years ago (2013-11-26 00:12:18 UTC) #6
commit-bot: I haz the power
7 years ago (2013-11-26 01:54:58 UTC) #7
Message was sent while issue was closed.
Change committed as 162651

Powered by Google App Engine
This is Rietveld 408576698