|
|
Chromium Code Reviews
DescriptionSelection API: deleteFromDocument() should refer to a registered Range.
This CL fixes 22 tests in external/wpt/selection/.
BUG=692400
Review-Url: https://codereview.chromium.org/2709483002
Cr-Commit-Position: refs/heads/master@{#451633}
Committed: https://chromium.googlesource.com/chromium/src/+/2db9622432f02cef305a8e7870793394992e2b41
Patch Set 1 #Patch Set 2 : rebase #
Messages
Total messages: 27 (17 generated)
The CQ bit was checked by tkent@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...
Description was changed from ========== Selection API: deleteFromDocument() should refer to a registered Range. BUG=692400 ========== to ========== Selection API: deleteFromDocument() should refer to a registered Range. This CL fixes 22 tests in external/wpt/selection/. BUG=692400 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
tkent@chromium.org changed reviewers: + yoichio@chromium.org, yosin@chromium.org
yosin@, yoichio@, would you review this please? fast/css/first-letter-rtc-crash.html fails on bots. I'll adjust test result later.
lgtm
On 2017/02/20 at 08:22:26, tkent wrote: > yosin@, yoichio@, would you review this please? > > fast/css/first-letter-rtc-crash.html fails on bots. I'll adjust test result later. I think it is OK to remove "first-letter-rtc-crash.html" since - it is generated by cluster fuzz - it is crash test for TextIterator::hasVisibleTextNode() and TextIteratorTest.cpp contains test for first-letter. - First-letter handling TextIterator was changed on Dec 2016 by http://crrev.com/2541163003
On 2017/02/20 at 08:44:26, yosin wrote: > On 2017/02/20 at 08:22:26, tkent wrote: > > yosin@, yoichio@, would you review this please? > > > > fast/css/first-letter-rtc-crash.html fails on bots. I'll adjust test result later. Fortunately we don't need to do rebase for it after the addRange() change.
The CQ bit was checked by tkent@chromium.org
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
Failed to apply patch for
third_party/WebKit/Source/core/editing/DOMSelection.cpp:
While running git apply --index -p1;
error: patch failed:
third_party/WebKit/Source/core/editing/DOMSelection.cpp:654
error: third_party/WebKit/Source/core/editing/DOMSelection.cpp: patch does not
apply
Patch: third_party/WebKit/Source/core/editing/DOMSelection.cpp
Index: third_party/WebKit/Source/core/editing/DOMSelection.cpp
diff --git a/third_party/WebKit/Source/core/editing/DOMSelection.cpp
b/third_party/WebKit/Source/core/editing/DOMSelection.cpp
index
bb6e173919ba4199fe6b7bb9a67fccceaed84d45..eb2c80fc524ae270e9cbdce2bc953b3f91412136
100644
--- a/third_party/WebKit/Source/core/editing/DOMSelection.cpp
+++ b/third_party/WebKit/Source/core/editing/DOMSelection.cpp
@@ -654,10 +654,24 @@ void DOMSelection::addRange(Range* newRange) {
cacheRangeIfSelectionOfDocument(createRange(merged));
}
+// https://www.w3.org/TR/selection-api/#dom-selection-deletefromdocument
void DOMSelection::deleteFromDocument() {
if (!isAvailable())
return;
+ // The method must invoke deleteContents() ([DOM4]) on the context object's
+ // range if the context object is not empty. Otherwise the method must do
+ // nothing.
+ if (Range* range = documentCachedRange()) {
+ range->deleteContents(ASSERT_NO_EXCEPTION);
+ return;
+ }
+
+ // The following code is necessary for
+ // editing/selection/deleteFromDocument-crash.html, which assumes
+ // deleteFromDocument() for text selection in a TEXTAREA deletes the TEXTAREA
+ // value.
+
FrameSelection& selection = frame()->selection();
if (selection.isNone())
The CQ bit was checked by tkent@chromium.org
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
Failed to apply patch for
third_party/WebKit/Source/core/editing/DOMSelection.cpp:
While running git apply --index -p1;
error: patch failed:
third_party/WebKit/Source/core/editing/DOMSelection.cpp:654
error: third_party/WebKit/Source/core/editing/DOMSelection.cpp: patch does not
apply
Patch: third_party/WebKit/Source/core/editing/DOMSelection.cpp
Index: third_party/WebKit/Source/core/editing/DOMSelection.cpp
diff --git a/third_party/WebKit/Source/core/editing/DOMSelection.cpp
b/third_party/WebKit/Source/core/editing/DOMSelection.cpp
index
bb6e173919ba4199fe6b7bb9a67fccceaed84d45..eb2c80fc524ae270e9cbdce2bc953b3f91412136
100644
--- a/third_party/WebKit/Source/core/editing/DOMSelection.cpp
+++ b/third_party/WebKit/Source/core/editing/DOMSelection.cpp
@@ -654,10 +654,24 @@ void DOMSelection::addRange(Range* newRange) {
cacheRangeIfSelectionOfDocument(createRange(merged));
}
+// https://www.w3.org/TR/selection-api/#dom-selection-deletefromdocument
void DOMSelection::deleteFromDocument() {
if (!isAvailable())
return;
+ // The method must invoke deleteContents() ([DOM4]) on the context object's
+ // range if the context object is not empty. Otherwise the method must do
+ // nothing.
+ if (Range* range = documentCachedRange()) {
+ range->deleteContents(ASSERT_NO_EXCEPTION);
+ return;
+ }
+
+ // The following code is necessary for
+ // editing/selection/deleteFromDocument-crash.html, which assumes
+ // deleteFromDocument() for text selection in a TEXTAREA deletes the TEXTAREA
+ // value.
+
FrameSelection& selection = frame()->selection();
if (selection.isNone())
The CQ bit was checked by tkent@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...
The CQ bit was unchecked by tkent@chromium.org
The CQ bit was checked by tkent@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/2709483002/#ps20001 (title: "rebase")
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": 20001, "attempt_start_ts": 1487605472692600,
"parent_rev": "bcc2246dd6c9f56186e23e11811d06707cde5735", "commit_rev":
"2db9622432f02cef305a8e7870793394992e2b41"}
Message was sent while issue was closed.
Description was changed from ========== Selection API: deleteFromDocument() should refer to a registered Range. This CL fixes 22 tests in external/wpt/selection/. BUG=692400 ========== to ========== Selection API: deleteFromDocument() should refer to a registered Range. This CL fixes 22 tests in external/wpt/selection/. BUG=692400 Review-Url: https://codereview.chromium.org/2709483002 Cr-Commit-Position: refs/heads/master@{#451633} Committed: https://chromium.googlesource.com/chromium/src/+/2db9622432f02cef305a8e787079... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/2db9622432f02cef305a8e787079... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
