Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2336043006:
Restore only the collapsed leading space when copy text (Closed)
|
Created:
4 years, 3 months ago by joone Modified:
4 years, 2 months ago Reviewers:
yosin_UTC9 CC:
blink-reviews, chromium-reviews Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionRestore 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
Messages
Total messages: 35 (21 generated)
The CQ bit was checked by joone.hur@intel.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Restore only the collapsed leading space when copy the text BUG=318925 TEST=TextIteratorTest.PreserveOnlyLeadingSpace ========== to ========== Restore only the collapsed leading space when copy the 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 ==========
Description was changed from ========== Restore only the collapsed leading space when copy the 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 ========== to ========== Restore only the collapsed leading space when copy the 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 ==========
Description was changed from ========== Restore only the collapsed leading space when copy the 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 ========== to ========== 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 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: 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_...)
joone.hur@intel.com changed reviewers: + yosin@chromium.org
Hi yosin@ please review this CL.
lgtm w/ nit https://codereview.chromium.org/2336043006/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/editing/iterators/TextIteratorTest.cpp (right): https://codereview.chromium.org/2336043006/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/editing/iterators/TextIteratorTest.cpp:540: Position start(div->firstChild()->firstChild()->firstChild(), 0); Could you use "id" attribute and |getElementById()| rather than cascading |firstChild()|?
The CQ bit was checked by joone.hur@intel.com
The patchset sent to the CQ was uploaded after l-g-t-m from yosin@chromium.org Link to the patchset: https://codereview.chromium.org/2336043006/#ps20001 (title: "use getElementById")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
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_...)
On 2016/09/15 19:14:35, commit-bot: I haz the power wrote: > 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_...) telemetry_perf_unittests (retry summary) telemetry_perf_unittests (retry summary) failures: benchmarks.system_health_smoke_test.SystemHealthBenchmarkSmokeTest.system_health.memory_mobile.load:news:wikipedia benchmarks.system_health_smoke_test.SystemHealthBenchmarkSmokeTest.system_health.memory_mobile.search:portal:google Standard output: ******************************************************************************** Cannot get standard output on Android ******************************************************************************** [ FAILED ] load:news:wikipedia (16788 ms) [ PASSED ] 0 tests. [ FAILED ] 1 test, listed below: [ FAILED ] load:news:wikipedia 1 FAILED TEST I'm trying to fix this problem.
This crash may delay the result.
--------- beginning of crash
09-15 23:27:53.494 21481 21494 F libc : Fatal signal 11 (SIGSEGV), code 1,
fault addr 0x0 in tid 21494 (CrRendererMain)
09-15 23:27:53.596 496 496 F DEBUG : *** *** *** *** *** *** *** ***
*** *** *** *** *** *** *** ***
09-15 23:27:53.596 496 496 F DEBUG : Build fingerprint:
'google/bullhead/bullhead:6.0.1/MMB29Q/2480792:userdebug/dev-keys'
09-15 23:27:53.596 496 496 F DEBUG : Revision: 'rev_1.0'
09-15 23:27:53.596 496 496 F DEBUG : ABI: 'arm64'
09-15 23:27:53.596 496 496 F DEBUG : pid: 21481, tid: 21494, name:
CrRendererMain >>> org.chromium.chrome:sandboxed_process0 <<<
09-15 23:27:53.596 496 496 F DEBUG : signal 11 (SIGSEGV), code 1
(SEGV_MAPERR), fault addr 0x0
09-15 23:27:53.610 496 496 F DEBUG : x0 0000000000000000 x1
0000000000000040 x2 0000007f701274e8 x3 0000000000000037
09-15 23:27:53.611 496 496 F DEBUG : x4 000000000000003a x5
0000000040100401 x6 0000000000000040 x7 0000007f8710d4f0
09-15 23:27:53.611 496 496 F DEBUG : x8 0000007f8710d440 x9
000000000000000d x10 0000000000000016 x11 0065006e0020006c
09-15 23:27:53.611 496 496 F DEBUG : x12 0074002000640065 x13
006500620020006f x14 201400a00022002e x15 003b9aca00000000
09-15 23:27:53.611 496 496 F DEBUG : x16 0000007f72f8adf0 x17
0000007f90353f5c x18 0000007f90420f50 x19 0000000000000001
09-15 23:27:53.611 496 496 F DEBUG : x20 00000010deb9b0f0 x21
0000007f8710af90 x22 0000000000000000 x23 0000000000000036
09-15 23:27:53.612 496 496 F DEBUG : x24 0000000000000036 x25
0000000000000036 x26 00000010dec6d4d0 x27 0000007f71470670
09-15 23:27:53.612 496 496 F DEBUG : x28 0000007f8710b350 x29
0000007f8710af00 x30 0000007f6febdda4
09-15 23:27:53.612 496 496 F DEBUG : sp 0000007f8710af00 pc
0000007f6febde4c pstate 0000000020000000
09-15 23:27:53.619 496 496 W debuggerd64: type=1400 audit(0.0:1438): avc:
denied { search } for name="org.chromium.chrome" dev="dm-2" ino=377159
scontext=u:r:debuggerd:s0 tcontext=u:object_r:app_data_file:s0:c512,c768
tclass=dir permissive=0
09-15 23:27:53.623 496 496 F DEBUG :
The CQ bit was checked by joone.hur@intel.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Hi yosin@ Could you take a look at the updated CL? I've fixed a crash issue on Android. Thanks! https://codereview.chromium.org/2336043006/diff/140001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp (right): https://codereview.chromium.org/2336043006/diff/140001/third_party/WebKit/Sou... third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp:623: if (firstBoxOfNextLine) |firstBoxOfNextLine| can be null on Android.
On 2016/09/16 05:04:13, joone wrote: > Hi yosin@ > > Could you take a look at the updated CL? > I've fixed a crash issue on Android. > > Thanks! > > https://codereview.chromium.org/2336043006/diff/140001/third_party/WebKit/Sou... > File third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp (right): > > https://codereview.chromium.org/2336043006/diff/140001/third_party/WebKit/Sou... > third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp:623: if > (firstBoxOfNextLine) > |firstBoxOfNextLine| can be null on Android. It's easier to see the change by comparing the patch set 2 and the latest one.
The CQ bit was checked by joone.hur@intel.com
The patchset sent to the CQ was uploaded after l-g-t-m from yosin@chromium.org Link to the patchset: https://codereview.chromium.org/2336043006/#ps140001 (title: "[For landing] Fix crash on Android")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
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_...)
The CQ bit was checked by joone.hur@intel.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== 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 ========== to ========== 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 ==========
Message was sent while issue was closed.
Committed patchset #8 (id:140001)
Message was sent while issue was closed.
Description was changed from ========== 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 ========== to ========== 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} ==========
Message was sent while issue was closed.
Patchset 8 (id:??) landed as https://crrev.com/b52f205cb0cf1d448a59192f484a3c14a832bc5e Cr-Commit-Position: refs/heads/master@{#421133}
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! |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
