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

Issue 2336043006: Restore only the collapsed leading space when copy text (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 months, 2 weeks ago by joone
Modified:
9 months ago
Reviewers:
yosin_UTC9
CC:
blink-reviews, chromium-reviews
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Restore only the collapsed leading space when copy text When we need to restore the trailing space of a line and the leading space of the next line together for copy operation, we don't have to restore the trailing space in the first line. Example: <div style="width: 2em;"><b><i>foo </i></b> bar</div> BUG=318925 TEST=TextIteratorTest.PreserveOnlyLeadingSpace Committed: https://crrev.com/b52f205cb0cf1d448a59192f484a3c14a832bc5e Cr-Commit-Position: refs/heads/master@{#421133}

Patch Set 1 #

Total comments: 1

Patch Set 2 : use getElementById #

Patch Set 3 : Try to fix the smoke test fails on Android n5x #

Patch Set 4 : [DoNotLand] #

Patch Set 5 : Try to fix the smoke test #

Patch Set 6 : [DoNotLand] Try to fix the crash on Android #

Patch Set 7 : [DoNotLand] Try without check() #

Patch Set 8 : [For landing] Fix crash on Android #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -2 lines) Patch
M third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp View 1 2 3 4 5 6 7 1 chunk +10 lines, -2 lines 1 comment Download
M third_party/WebKit/Source/core/editing/iterators/TextIteratorTest.cpp View 1 1 chunk +9 lines, -0 lines 0 comments Download
Trybot results:  win_clang   win_chromium_compile_dbg_ng   win_chromium_x64_rel_ng   win_chromium_rel_ng   mac_chromium_compile_dbg_ng   ios-device   ios-simulator   mac_chromium_rel_ng   linux_android_rel_ng   android_n5x_swarming_rel   android_clang_dbg_recipe   android_arm64_dbg_recipe   android_compile_dbg   cast_shell_android   linux_chromium_chromeos_compile_dbg_ng   linux_chromium_asan_rel_ng   linux_chromium_rel_ng   chromeos_x86-generic_chromium_compile_only_ng   chromium_presubmit   linux_chromium_compile_dbg_ng   chromeos_amd64-generic_chromium_compile_only_ng   cast_shell_linux   chromeos_daisy_chromium_compile_only_ng   linux_chromium_chromeos_ozone_rel_ng   linux_chromium_chromeos_rel_ng   linux_chromium_clobber_rel_ng   linux_android_rel_ng   win_chromium_x64_rel_ng   linux_android_rel_ng   win_chromium_x64_rel_ng   linux_android_rel_ng   win_chromium_x64_rel_ng   win_clang   win_chromium_x64_rel_ng   win_chromium_compile_dbg_ng   mac_chromium_compile_dbg_ng   win_chromium_rel_ng   mac_chromium_rel_ng   ios-simulator   ios-device   linux_android_rel_ng   android_n5x_swarming_rel   android_clang_dbg_recipe   android_arm64_dbg_recipe   android_compile_dbg   cast_shell_android   linux_chromium_chromeos_compile_dbg_ng   linux_chromium_asan_rel_ng   chromeos_x86-generic_chromium_compile_only_ng   linux_chromium_rel_ng   chromium_presubmit   linux_chromium_compile_dbg_ng   cast_shell_linux   chromeos_amd64-generic_chromium_compile_only_ng   chromeos_daisy_chromium_compile_only_ng   linux_chromium_chromeos_ozone_rel_ng   linux_chromium_clobber_rel_ng   linux_chromium_chromeos_rel_ng   win_clang   win_chromium_compile_dbg_ng   win_chromium_rel_ng   win_chromium_x64_rel_ng   mac_chromium_compile_dbg_ng   ios-device   ios-simulator   linux_android_rel_ng   mac_chromium_rel_ng   android_n5x_swarming_rel   android_clang_dbg_recipe   android_arm64_dbg_recipe   android_compile_dbg   cast_shell_android   linux_chromium_asan_rel_ng   linux_chromium_chromeos_compile_dbg_ng   chromeos_x86-generic_chromium_compile_only_ng   linux_chromium_rel_ng   linux_chromium_compile_dbg_ng   chromium_presubmit   chromeos_amd64-generic_chromium_compile_only_ng   cast_shell_linux   chromeos_daisy_chromium_compile_only_ng   linux_chromium_chromeos_ozone_rel_ng   linux_chromium_chromeos_rel_ng   linux_chromium_clobber_rel_ng 
Commit queue not available (can’t edit this change).

Messages

Total messages: 35 (21 generated)
joone
Hi yosin@ please review this CL.
9 months, 2 weeks ago (2016-09-15 00:31:40 UTC) #9
yosin_UTC9
lgtm w/ nit https://codereview.chromium.org/2336043006/diff/1/third_party/WebKit/Source/core/editing/iterators/TextIteratorTest.cpp File third_party/WebKit/Source/core/editing/iterators/TextIteratorTest.cpp (right): https://codereview.chromium.org/2336043006/diff/1/third_party/WebKit/Source/core/editing/iterators/TextIteratorTest.cpp#newcode540 third_party/WebKit/Source/core/editing/iterators/TextIteratorTest.cpp:540: Position start(div->firstChild()->firstChild()->firstChild(), 0); Could you use ...
9 months, 2 weeks ago (2016-09-15 01:41:12 UTC) #10
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/2336043006/20001
9 months, 2 weeks ago (2016-09-15 17:45:06 UTC) #13
commit-bot: I haz the power
Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_swarming_rel/builds/31365)
9 months, 2 weeks ago (2016-09-15 19:14:35 UTC) #15
joone
On 2016/09/15 19:14:35, commit-bot: I haz the power wrote: > Try jobs failed on following ...
9 months, 2 weeks ago (2016-09-15 22:57:40 UTC) #16
joone
This crash may delay the result. --------- beginning of crash 09-15 23:27:53.494 21481 21494 F ...
9 months, 2 weeks ago (2016-09-15 23:52:56 UTC) #17
joone
Hi yosin@ Could you take a look at the updated CL? I've fixed a crash ...
9 months, 2 weeks ago (2016-09-16 05:04:13 UTC) #22
joone
On 2016/09/16 05:04:13, joone wrote: > Hi yosin@ > > Could you take a look ...
9 months, 1 week ago (2016-09-19 18:14:04 UTC) #23
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/2336043006/140001
9 months ago (2016-09-26 04:41:50 UTC) #26
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_rel_ng/builds/285876)
9 months ago (2016-09-26 06:03:43 UTC) #28
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/2336043006/140001
9 months ago (2016-09-27 06:04:28 UTC) #30
commit-bot: I haz the power
Committed patchset #8 (id:140001)
9 months ago (2016-09-27 07:13:39 UTC) #32
commit-bot: I haz the power
Patchset 8 (id:??) landed as https://crrev.com/b52f205cb0cf1d448a59192f484a3c14a832bc5e Cr-Commit-Position: refs/heads/master@{#421133}
9 months ago (2016-09-27 07:16:11 UTC) #34
joone
9 months ago (2016-09-27 07:24:46 UTC) #35
Message was sent while issue was closed.
On 2016/09/27 07:16:11, commit-bot: I haz the power wrote:
> Patchset 8 (id:??) landed as
> https://crrev.com/b52f205cb0cf1d448a59192f484a3c14a832bc5e
> Cr-Commit-Position: refs/heads/master@{#421133}

Hi yosin@ please let me know if this commit needs any change. Thank you!
Sign in to reply to this message.

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