This change is ready for review.
To view, visit change 620329. To unsubscribe, or for help writing mail filters, visit settings.
1 comment:
File third_party/WebKit/LayoutTests/editing/execCommand/list-wrapping-image-crash.html:
Patch Set #1, Line 9: selection.document.execCommand('insertimage');
Please use input text after execCommand('insertImage') to make
tester parameter of |assert_selection()| to 'insertOrderedList' only.
To view, visit change 620329. To unsubscribe, or for help writing mail filters, visit settings.
1 comment:
File third_party/WebKit/LayoutTests/editing/execCommand/list-wrapping-image-crash.html:
Please use input text after execCommand('insertImage') to make […]
Done
To view, visit change 620329. To unsubscribe, or for help writing mail filters, visit settings.
LGTM with a nit.
Patch set 3:Code-Review +1
1 comment:
File third_party/WebKit/LayoutTests/editing/execCommand/list-wrapping-image-crash.html:
Patch Set #3, Line 8: 'insertorderedlist',
nit: insertOrderList.
We prefer kCamelCase style in JavaScript.
To view, visit change 620329. To unsubscribe, or for help writing mail filters, visit settings.
Fixed the selection command to make it follow C++ naming style.
1 comment:
File third_party/WebKit/LayoutTests/editing/execCommand/list-wrapping-image-crash.html:
Patch Set #3, Line 8: 'insertOrderedList',
nit: insertOrderList. […]
Done
To view, visit change 620329. To unsubscribe, or for help writing mail filters, visit settings.
Patch set 4:Commit-Queue +2
CQ is trying da patch.
Note: The patchset sent to CQ was uploaded after this CL was approved.
"Fixed the selection to follow JS naming style" https://chromium-review.googlesource.com/c/620329/4
Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/chromium-review.googlesource.com/620329/4
Bot data: {"action": "start", "triggered_at": "2017-08-21T01:31:01.0Z", "cq_cfg_revision": "81f1a3f0cb07446abeff7dd01ff1b8bae94ffe0d", "revision": "f9e62001d52953491019fa28a0a0d4ac315b5e0e"}
Commit Bot merged this change.
Converts editing/execCommand/list-wrapping-image-crash.html to utilize assert_selection().
This patch converts "editing/execCommand/list-wrapping-image-crash.html"
to utilize |assert_selection()| for ease of maintaining this test
script to provide a hint of what this test script, for improving code health.
Bug: 753289, 679977
Change-Id: I499a669165d61f8644bcf62fca3d71f9b4d9b027
Reviewed-on: https://chromium-review.googlesource.com/620329
Commit-Queue: Akari Asai <akar...@google.com>
Reviewed-by: Xiaocheng Hu <xiaoc...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495862}
---
D third_party/WebKit/LayoutTests/editing/execCommand/list-wrapping-image-crash-expected.txt
M third_party/WebKit/LayoutTests/editing/execCommand/list-wrapping-image-crash.html
2 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/third_party/WebKit/LayoutTests/editing/execCommand/list-wrapping-image-crash-expected.txt b/third_party/WebKit/LayoutTests/editing/execCommand/list-wrapping-image-crash-expected.txt
deleted file mode 100644
index fb37d38..0000000
--- a/third_party/WebKit/LayoutTests/editing/execCommand/list-wrapping-image-crash-expected.txt
+++ /dev/null
@@ -1 +0,0 @@
-PASSED - this test case didn't ASSERT, bug 19066
diff --git a/third_party/WebKit/LayoutTests/editing/execCommand/list-wrapping-image-crash.html b/third_party/WebKit/LayoutTests/editing/execCommand/list-wrapping-image-crash.html
index 2414d7e..2abcf40 100644
--- a/third_party/WebKit/LayoutTests/editing/execCommand/list-wrapping-image-crash.html
+++ b/third_party/WebKit/LayoutTests/editing/execCommand/list-wrapping-image-crash.html
@@ -1,10 +1,17 @@
-<body>
+<!doctype html>
+<script src="../../resources/testharness.js"></script>
+<script src="../../resources/testharnessreport.js"></script>
+<script src="../assert_selection.js"></script>
<script>
-if (window.testRunner)
- testRunner.dumpAsText();
-document.designMode = "on";
-document.execCommand('selectall');
-document.execCommand('insertimage');
-document.execCommand('insertorderedlist');
-document.body.innerHTML = "PASSED - this test case didn't ASSERT, bug 19066";
+test(()=> assert_selection(
+ '<div contenteditable><img>|</div>',
+ 'insertOrderedList',
+ [
+ "<div contenteditable>",
+ "<ol>",
+ "<li><img>|</li>",
+ "</ol>",
+ "</div>"]),
+ 'This insertOrderedList on an image in an editable region without selection change.');
</script>
+
To view, visit change 620329. To unsubscribe, or for help writing mail filters, visit settings.
This change is ready for review.
To view, visit change 632197. To unsubscribe, or for help writing mail filters, visit settings.
Patch set 5:Code-Review +1
3 comments:
File third_party/WebKit/LayoutTests/editing/execCommand/insert-paragraph-into-table.html:
Patch Set #5, Line 6: https://bugs.chromium.org/p/chromium/issues/detail?id=535510
nit: Use crbug.com/535510.
"<div contenteditable>",
"<table>",
"<tbody>",
"<td style='display:inline-block'>",
"<ruby><rt>a|</rt></ruby>",
"</td>",
"</tbody>",
"</table>^",
"</div>"
Use single quote for the outer quotes, and double quote as the inner ones.
"<div contenteditable>",
"<table>",
"<tbody>",
"<tr><td style=\"display:inline-block\">",
"<ruby><rt>a</rt></ruby>",
"</td></tr>",
"<tr><span style=\"font-size: 8px;\">|<br></span></tr>",
"</tbody>",
"</table>",
"</div>"
Use single quote for the outer quotes, and double quote as the inner ones.
In this you you don't need to write the escape sequence \" inside the strings.
To view, visit change 632197. To unsubscribe, or for help writing mail filters, visit settings.
3 comments:
File third_party/WebKit/LayoutTests/editing/execCommand/insert-paragraph-into-table.html:
Patch Set #5, Line 6: crbug.com/535510
nit: Use crbug.com/535510.
Done
'<div contenteditable>',
'<table>',
'<tbody>',
'<td style="display:inline-block">',
'<ruby><rt>a|</rt></ruby>',
'</td>',
'</tbody>',
'</table>^',
'</div>'
Use single quote for the outer quotes, and double quote as the inner ones.
Done
'<div contenteditable>',
'<table>',
'<tbody>',
'<tr><td style="display:inline-block">',
'<ruby><rt>a</rt></ruby>',
'</td></tr>',
'<tr><span style="font-size: 8px;">|<br></span></tr>',
'</tbody>',
'</table>',
'</div>'
Use single quote for the outer quotes, and double quote as the inner ones. […]
Done
To view, visit change 632197. To unsubscribe, or for help writing mail filters, visit settings.
lgtm
Patch set 7:Code-Review +1Commit-Queue +2
Commit Bot merged this change.
Converts editing/execCommand/list-wrapping-image-crash.html to utilize assert_selection().
This patch converts "editing/execCommand/insert-paragraph-into-table.html"
to utilize |assert_selection()| for ease of maintaining this test
script to provide a hint of what this test script, for improving code health.
Bug: 753289, 679977
Change-Id: I5d5af3e1fbed930b35ce8d0b9f8c8a7b97271364
Reviewed-on: https://chromium-review.googlesource.com/632197
Commit-Queue: Yoshifumi Inoue <yo...@chromium.org>
Reviewed-by: Yoshifumi Inoue <yo...@chromium.org>
Reviewed-by: Xiaocheng Hu <xiaoc...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498428}
---
M third_party/WebKit/LayoutTests/editing/execCommand/insert-paragraph-into-table.html
1 file changed, 28 insertions(+), 13 deletions(-)
diff --git a/third_party/WebKit/LayoutTests/editing/execCommand/insert-paragraph-into-table.html b/third_party/WebKit/LayoutTests/editing/execCommand/insert-paragraph-into-table.html
index 8727d5e..6f1a147 100644
--- a/third_party/WebKit/LayoutTests/editing/execCommand/insert-paragraph-into-table.html
+++ b/third_party/WebKit/LayoutTests/editing/execCommand/insert-paragraph-into-table.html
@@ -1,17 +1,32 @@
-<!DOCTYPE html>
+<!doctype html>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
-<div id="sample" contenteditable>
-<table><td style="display:inline-block"><ruby><rt>a</rt></ruby></td></table>
-</div>
-<div id="log"></div>
+<script src="../assert_selection.js"></script>
<script>
-test(function() {
- var selection = window.getSelection();
- var sample = document.getElementById('sample');
- selection.collapse(sample, 2);
- selection.extend(document.querySelector('rt').firstChild, 1);
- document.execCommand('InsertParagraph');
- assert_equals(sample.innerHTML.replace(/\n/g, ''), '<table><tbody><tr><td style="display:inline-block"><ruby><rt>a</rt></ruby></td></tr><tr><font size="1"><br></font></tr></tbody></table>');
-});
+//This is a test case for crbug.com/535510
+test(()=> assert_selection(
+ [
+ '<div contenteditable>',
+ '<table>',
+ '<tbody>',
+ '<td style="display:inline-block">',
+ '<ruby><rt>a|</rt></ruby>',
+ '</td>',
+ '</tbody>',
+ '</table>^',
+ '</div>'],
+ 'InsertParagraph',
+ [
+ '<div contenteditable>',
+ '<table>',
+ '<tbody>',
+ '<tr><td style="display:inline-block">',
+ '<ruby><rt>a</rt></ruby>',
+ '</td></tr>',
+ '<tr><font size="1">|<br></font></tr>',
+ '</tbody>',
+ '</table>',
+ '</div>'
+ ]),
+ "This InsertParagraph non-table layout item into table layout.");
</script>
To view, visit change 632197. To unsubscribe, or for help writing mail filters, visit settings.