Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2738203002:
Update EphemeralRange Unit Tests (Closed)
DescriptionUpdate EphemeralRange Unit Tests
EphemeralRange unit tests updated with better HTML and shadow-DOM
attached explicitly. Previously few of the test cases introduced
extra dependency on the UA shadow DOM under <input>,
which was irrelevant to EphemeralRange.
BUG=697989
Review-Url: https://codereview.chromium.org/2738203002
Cr-Commit-Position: refs/heads/master@{#456184}
Committed: https://chromium.googlesource.com/chromium/src/+/7cb26a09a58fbbf6dc28d3a475757445756fd46f
Patch Set 1 #Patch Set 2 : Y #
Total comments: 9
Patch Set 3 : Y #Messages
Total messages: 23 (13 generated)
Description was changed from ========== Unit tests in EphemeralRangeTest.cpp are badly written EphemeralRange unit tests updated with better HTML and shadow Doms attached explicitly. BUG=697989 ========== to ========== Unit tests in EphemeralRangeTest.cpp are badly written EphemeralRange unit tests updated with better HTML and shadow Doms attached explicitly. BUG=697989 R=SamsungPeerReview ==========
tanvir.rizvi@samsung.com changed reviewers: + shanmuga.m@samsung.com
On 2017/03/09 11:00:09, tanvir wrote: Please Follow guideline for description. https://www.chromium.org/developers/contributing-code#TOC-Writing-change-list...
Description was changed from
==========
Unit tests in EphemeralRangeTest.cpp are badly written
EphemeralRange unit tests updated with better HTML and shadow Doms attached
explicitly.
BUG=697989
R=SamsungPeerReview
==========
to
==========
Update EphemeralRange Unit Tests
EphemeralRange unit tests updated with better HTML and shadow-DOM
attached explicitly. Previously few of the test cases introduced
extra dependency on the UA shadow DOM under <input>,
which was irrelevant to EphemeralRange.
BUG=697989
R=SamsungPeerReview
==========
On 2017/03/09 11:29:43, Shanmuga Pandi wrote: > On 2017/03/09 11:00:09, tanvir wrote: > > Please Follow guideline for description. > https://www.chromium.org/developers/contributing-code#TOC-Writing-change-list... Done
On 2017/03/09 14:19:50, tanvir wrote: > On 2017/03/09 11:29:43, Shanmuga Pandi wrote: > > On 2017/03/09 11:00:09, tanvir wrote: > > > > Please Follow guideline for description. > > > https://www.chromium.org/developers/contributing-code#TOC-Writing-change-list... > > Done Peer review looks good to me.
Description was changed from
==========
Update EphemeralRange Unit Tests
EphemeralRange unit tests updated with better HTML and shadow-DOM
attached explicitly. Previously few of the test cases introduced
extra dependency on the UA shadow DOM under <input>,
which was irrelevant to EphemeralRange.
BUG=697989
R=SamsungPeerReview
==========
to
==========
Update EphemeralRange Unit Tests
EphemeralRange unit tests updated with better HTML and shadow-DOM
attached explicitly. Previously few of the test cases introduced
extra dependency on the UA shadow DOM under <input>,
which was irrelevant to EphemeralRange.
BUG=697989
==========
tanvir.rizvi@samsung.com changed reviewers: + xiaochengh@chromium.org, yosin@chromium.org
lgtm Thanks for quick work!
https://codereview.chromium.org/2738203002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp (right): https://codereview.chromium.org/2738203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp:60: "<span id='three'>" Could you add a closing tag to this <span>, and make it non-empty? https://codereview.chromium.org/2738203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp:84: "<b id='two'>22</b>" Let's add more node under #host and verify that it doesn't appear in flat tree without being inserted into a <content> or <slot>. https://codereview.chromium.org/2738203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp:91: "<span id='five'>" ditto. https://codereview.chromium.org/2738203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp:113: "<span id='three'>" ditto. https://codereview.chromium.org/2738203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp:147: "<span id='five'>" ditto.
Changes Done. PTAL!!! Thanks. https://codereview.chromium.org/2738203002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp (right): https://codereview.chromium.org/2738203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp:60: "<span id='three'>" On 2017/03/10 01:27:15, Xiaocheng wrote: > Could you add a closing tag to this <span>, and make it non-empty? Done. https://codereview.chromium.org/2738203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp:84: "<b id='two'>22</b>" On 2017/03/10 01:27:15, Xiaocheng wrote: > Let's add more node under #host and verify that it doesn't appear in flat tree > without being inserted into a <content> or <slot>. Done. https://codereview.chromium.org/2738203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp:113: "<span id='three'>" On 2017/03/10 01:27:16, Xiaocheng wrote: > ditto. Done. https://codereview.chromium.org/2738203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp:147: "<span id='five'>" On 2017/03/10 01:27:15, Xiaocheng wrote: > ditto. Done.
The CQ bit was checked by xiaochengh@chromium.org 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...
lgtm Thanks for working on this!
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by xiaochengh@chromium.org
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/2738203002/#ps40001 (title: "Y")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489183657355540,
"parent_rev": "786f4f5950e27410960fcb188dd4db4af1575656", "commit_rev":
"7cb26a09a58fbbf6dc28d3a475757445756fd46f"}
Message was sent while issue was closed.
Description was changed from
==========
Update EphemeralRange Unit Tests
EphemeralRange unit tests updated with better HTML and shadow-DOM
attached explicitly. Previously few of the test cases introduced
extra dependency on the UA shadow DOM under <input>,
which was irrelevant to EphemeralRange.
BUG=697989
==========
to
==========
Update EphemeralRange Unit Tests
EphemeralRange unit tests updated with better HTML and shadow-DOM
attached explicitly. Previously few of the test cases introduced
extra dependency on the UA shadow DOM under <input>,
which was irrelevant to EphemeralRange.
BUG=697989
Review-Url: https://codereview.chromium.org/2738203002
Cr-Commit-Position: refs/heads/master@{#456184}
Committed:
https://chromium.googlesource.com/chromium/src/+/7cb26a09a58fbbf6dc28d3a47575...
==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/7cb26a09a58fbbf6dc28d3a47575... |
