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

Issue 2725603002: createFragmentFromMarkupWithContext() should use EphemeralRange. (Closed)

Created:
3 years, 9 months ago by tanvir
Modified:
3 years, 9 months ago
CC:
chromium-reviews
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

createFragmentFromMarkupWithContext() should use EphemeralRange. Use EphemeralRange instead of Range in createFragmentFromMarkupWithContext() BUG=691201 Review-Url: https://codereview.chromium.org/2725603002 Cr-Commit-Position: refs/heads/master@{#455087} Committed: https://chromium.googlesource.com/chromium/src/+/31211cb8a35a8d9f6664a0f1f6d1957576e077e3

Patch Set 1 #

Total comments: 7

Patch Set 2 : Y #

Total comments: 11

Patch Set 3 : Y #

Total comments: 4

Patch Set 4 : Y #

Total comments: 5

Patch Set 5 : Y #

Patch Set 6 : Addressed Review Comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+59 lines, -3 lines) Patch
M AUTHORS View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/editing/EphemeralRange.h View 1 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/editing/EphemeralRange.cpp View 1 2 2 chunks +17 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp View 1 2 3 4 5 1 chunk +37 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/editing/serializers/Serialization.cpp View 1 2 3 4 1 chunk +2 lines, -3 lines 0 comments Download

Messages

Total messages: 42 (20 generated)
Shanmuga Pandi
Peer review looks good to me.
3 years, 9 months ago (2017-02-28 14:24:15 UTC) #7
tanvir
PTAL! Thanks
3 years, 9 months ago (2017-02-28 14:27:40 UTC) #10
Xiaocheng
Thanks for the patch! Guess you were warned of "needs formatting" during patch uploading. Could ...
3 years, 9 months ago (2017-02-28 19:26:31 UTC) #11
yosin_UTC9
On 2017/02/28 at 14:27:40, tanvir.rizvi wrote: > PTAL! > Thanks Please follow: Dry run: The ...
3 years, 9 months ago (2017-03-02 03:41:34 UTC) #16
tanvir
On 2017/03/02 03:41:34, yosin_UTC9 wrote: > On 2017/02/28 at 14:27:40, tanvir.rizvi wrote: > > PTAL! ...
3 years, 9 months ago (2017-03-02 14:56:27 UTC) #17
tanvir
Changes are done. PTAL! Thanks https://codereview.chromium.org/2725603002/diff/1/third_party/WebKit/Source/core/editing/EphemeralRange.cpp File third_party/WebKit/Source/core/editing/EphemeralRange.cpp (right): https://codereview.chromium.org/2725603002/diff/1/third_party/WebKit/Source/core/editing/EphemeralRange.cpp#newcode122 third_party/WebKit/Source/core/editing/EphemeralRange.cpp:122: return containerA->commonAncestor(*containerB, NodeTraversal::parent); On ...
3 years, 9 months ago (2017-03-02 14:59:14 UTC) #18
Xiaocheng
https://codereview.chromium.org/2725603002/diff/20001/third_party/WebKit/Source/core/editing/EphemeralRange.cpp File third_party/WebKit/Source/core/editing/EphemeralRange.cpp (right): https://codereview.chromium.org/2725603002/diff/20001/third_party/WebKit/Source/core/editing/EphemeralRange.cpp#newcode15 third_party/WebKit/Source/core/editing/EphemeralRange.cpp:15: nit: remove this extra blank line. https://codereview.chromium.org/2725603002/diff/20001/third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp File third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp ...
3 years, 9 months ago (2017-03-02 20:00:16 UTC) #19
tanvir
Changes are done. PTAL https://codereview.chromium.org/2725603002/diff/20001/third_party/WebKit/Source/core/editing/EphemeralRange.cpp File third_party/WebKit/Source/core/editing/EphemeralRange.cpp (right): https://codereview.chromium.org/2725603002/diff/20001/third_party/WebKit/Source/core/editing/EphemeralRange.cpp#newcode15 third_party/WebKit/Source/core/editing/EphemeralRange.cpp:15: On 2017/03/02 20:00:16, Xiaocheng wrote: ...
3 years, 9 months ago (2017-03-04 13:55:32 UTC) #20
Xiaocheng
Please revise the test cases accordingly. No more comments beyond that. https://codereview.chromium.org/2725603002/diff/40001/third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp File third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp (right): ...
3 years, 9 months ago (2017-03-05 06:31:19 UTC) #21
tanvir
Changes Done. PTAL https://codereview.chromium.org/2725603002/diff/40001/third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp File third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp (right): https://codereview.chromium.org/2725603002/diff/40001/third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp#newcode157 third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp:157: Range* range = getBodyRange(); On 2017/03/05 ...
3 years, 9 months ago (2017-03-06 07:05:11 UTC) #22
Xiaocheng
lgtm with a nit. https://codereview.chromium.org/2725603002/diff/60001/third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp File third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp (right): https://codereview.chromium.org/2725603002/diff/60001/third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp#newcode171 third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp:171: nit: remove this extra blank ...
3 years, 9 months ago (2017-03-06 07:33:21 UTC) #23
yosin_UTC9
https://codereview.chromium.org/2725603002/diff/60001/third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp File third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp (right): https://codereview.chromium.org/2725603002/diff/60001/third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp#newcode153 third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp:153: "<p id='host'>00<b id='one'>11</b><b id='two'>22</b><b " Could you use multiple ...
3 years, 9 months ago (2017-03-06 07:35:30 UTC) #24
tanvir
Changes done as per comments. PTAL!!! Thanks https://codereview.chromium.org/2725603002/diff/60001/third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp File third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp (right): https://codereview.chromium.org/2725603002/diff/60001/third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp#newcode153 third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp:153: "<p id='host'>00<b ...
3 years, 9 months ago (2017-03-06 14:42:01 UTC) #25
yosin_UTC9
lgtm Thanks for working this! https://codereview.chromium.org/2725603002/diff/60001/third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp File third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp (right): https://codereview.chromium.org/2725603002/diff/60001/third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp#newcode153 third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp:153: "<p id='host'>00<b id='one'>11</b><b id='two'>22</b><b ...
3 years, 9 months ago (2017-03-07 01:55:02 UTC) #26
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/2725603002/100001
3 years, 9 months ago (2017-03-07 01:56:34 UTC) #29
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/379008)
3 years, 9 months ago (2017-03-07 05:00:31 UTC) #31
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/2725603002/100001
3 years, 9 months ago (2017-03-07 05:45:57 UTC) #33
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/395501)
3 years, 9 months ago (2017-03-07 07:54:01 UTC) #35
tanvir
On 2017/03/07 07:54:01, commit-bot: I haz the power wrote: > Try jobs failed on following ...
3 years, 9 months ago (2017-03-07 10:22:28 UTC) #36
yosin_UTC9
On 2017/03/07 at 10:22:28, tanvir.rizvi wrote: > On 2017/03/07 07:54:01, commit-bot: I haz the power ...
3 years, 9 months ago (2017-03-07 14:38:14 UTC) #37
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/2725603002/100001
3 years, 9 months ago (2017-03-07 14:39:04 UTC) #39
commit-bot: I haz the power
3 years, 9 months ago (2017-03-07 16:08:33 UTC) #42
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/31211cb8a35a8d9f6664a0f1f6d1...

Powered by Google App Engine
This is Rietveld 408576698