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

Issue 2175163004: Add a plain space instead of   between text nodes (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 months, 1 week ago by joone
Modified:
11 months, 1 week 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.

Description

Add a plain space instead of   between text nodes When we rebalance white spaces,   can be added as space under some conditions. This CL adds a condition that the next sibling text node should not exist. BUG=310149 TEST=editing/inserting/insert-space.html Committed: https://crrev.com/8134eeb454ef76c72df8cd7c26f6141072c314cb Cr-Commit-Position: refs/heads/master@{#407981}

Patch Set 1 #

Total comments: 9

Patch Set 2 : use of assert_selection() and introduce |shouldEmitNBSPbeforeEnd| #

Total comments: 1

Patch Set 3 : don't need to use |Range| #

Patch Set 4 : fix a test fail in Win #

Unified diffs Side-by-side diffs Delta from patch set Stats (+39 lines, -8 lines) Patch
M third_party/WebKit/LayoutTests/TestExpectations View 1 2 3 1 chunk +5 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/editing/inserting/insert-space.html View 1 2 1 chunk +24 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/editing/pasteboard/paste-text-003-expected.txt View 3 chunks +3 lines, -3 lines 0 comments Download
M third_party/WebKit/LayoutTests/editing/pasteboard/smart-paste-003-trailing-whitespace-expected.txt View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/editing/EditingUtilities.h View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/editing/EditingUtilities.cpp View 1 2 3 2 chunks +3 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.cpp View 1 1 chunk +2 lines, -1 line 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 36 (24 generated)
joone
Hi yosin@ please review this CL.
11 months, 1 week ago (2016-07-25 07:19:08 UTC) #5
joone
Hi yosin@ please review this CL.
11 months, 1 week ago (2016-07-25 07:19:08 UTC) #6
yosin_UTC9
https://codereview.chromium.org/2175163004/diff/1/third_party/WebKit/LayoutTests/editing/inserting/insert-space.html File third_party/WebKit/LayoutTests/editing/inserting/insert-space.html (right): https://codereview.chromium.org/2175163004/diff/1/third_party/WebKit/LayoutTests/editing/inserting/insert-space.html#newcode18 third_party/WebKit/LayoutTests/editing/inserting/insert-space.html:18: assert_equals(html_para, '<p id="p1">A B</p>'); Can we use assert_selection()? https://codereview.chromium.org/2175163004/diff/1/third_party/WebKit/LayoutTests/editing/inserting/insert-space.html#newcode34 ...
11 months, 1 week ago (2016-07-25 07:42:23 UTC) #9
joone
Thanks for review! https://codereview.chromium.org/2175163004/diff/1/third_party/WebKit/LayoutTests/editing/inserting/insert-space.html File third_party/WebKit/LayoutTests/editing/inserting/insert-space.html (right): https://codereview.chromium.org/2175163004/diff/1/third_party/WebKit/LayoutTests/editing/inserting/insert-space.html#newcode18 third_party/WebKit/LayoutTests/editing/inserting/insert-space.html:18: assert_equals(html_para, '<p id="p1">A B</p>'); On 2016/07/25 ...
11 months, 1 week ago (2016-07-26 04:16:34 UTC) #19
yosin_UTC9
lgtm w/ small nit https://codereview.chromium.org/2175163004/diff/40001/third_party/WebKit/LayoutTests/editing/inserting/insert-space.html File third_party/WebKit/LayoutTests/editing/inserting/insert-space.html (right): https://codereview.chromium.org/2175163004/diff/40001/third_party/WebKit/LayoutTests/editing/inserting/insert-space.html#newcode23 third_party/WebKit/LayoutTests/editing/inserting/insert-space.html:23: selection.addRange(range); We don't need to ...
11 months, 1 week ago (2016-07-26 04:49:18 UTC) #20
yosin_UTC9
lgtm w/ small nit https://codereview.chromium.org/2175163004/diff/40001/third_party/WebKit/LayoutTests/editing/inserting/insert-space.html File third_party/WebKit/LayoutTests/editing/inserting/insert-space.html (right): https://codereview.chromium.org/2175163004/diff/40001/third_party/WebKit/LayoutTests/editing/inserting/insert-space.html#newcode23 third_party/WebKit/LayoutTests/editing/inserting/insert-space.html:23: selection.addRange(range); We don't need to ...
11 months, 1 week ago (2016-07-26 04:49:19 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2175163004/60001
11 months, 1 week ago (2016-07-26 06:01:45 UTC) #25
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/262051)
11 months, 1 week ago (2016-07-26 07:22:05 UTC) #27
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2175163004/60001
11 months, 1 week ago (2016-07-26 20:43:26 UTC) #29
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2175163004/80001
11 months, 1 week ago (2016-07-26 22:15:20 UTC) #32
commit-bot: I haz the power
Committed patchset #4 (id:80001)
11 months, 1 week ago (2016-07-27 00:05:14 UTC) #34
commit-bot: I haz the power
11 months, 1 week ago (2016-07-27 00:06:37 UTC) #36
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/8134eeb454ef76c72df8cd7c26f6141072c314cb
Cr-Commit-Position: refs/heads/master@{#407981}
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 23e94e589