third_party/WebKit/LayoutTests/editing/selection/user-select/user-select-all-contenteditable.html - Issue 2120913002: [Editing][Regression] Contenteditable w/ "-webkit-user-select:all" should be editable.

Side by Side Diff: third_party/WebKit/LayoutTests/editing/selection/user-select/user-select-all-contenteditable.html

Issue 2120913002: [Editing][Regression] Contenteditable w/ "-webkit-user-select:all" should be editable. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: update Created 4 years, 5 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

OLD	NEW
(Empty)
	1 <!doctype HTML>

	2 <script src="../../../resources/testharness.js"></script>

	3 <script src="../../../resources/testharnessreport.js"></script>

	4 <script src="../../assert_selection.js"></script>

	5 <div id="log"></div>

	6 <script>

	7 test(function(){

	8 assert_selection(

	9 '<div contenteditable="true" id="div" style="-webkit-user-select:all">fo o</div>',

	10 selection => {

	11 selection.collapse(selection.document.getElementById('div'), 0);
	yosin_UTC9 2016/07/07 08:59:48 Why not put "\|" in test sample? Why not put "\|" in test sample? yoichio 2016/07/07 09:03:27 That's because "putting a caret inside the element Show quoted text On 2016/07/07 08:59:48, Yosi_UTC9 wrote: > Why not put "\|" in test sample? That's because "putting a caret inside the element" is a part of test. yosin_UTC9 2016/07/08 01:15:04 assert_selection() does for you. It is Selection#c Show quoted text On 2016/07/07 at 09:03:27, yoichio wrote: > On 2016/07/07 08:59:48, Yosi_UTC9 wrote: > > Why not put "\|" in test sample? > > That's because "putting a caret inside the element" is a part of test. assert_selection() does for you. It is Selection#collapse() and Selection#extend(). Doing this is redundant. yoichio 2016/07/08 02:30:39 This is not redundant. assert_selection("foo\|bar", Show quoted text On 2016/07/08 01:15:04, Yosi_UTC9 wrote: > On 2016/07/07 at 09:03:27, yoichio wrote: > > On 2016/07/07 08:59:48, Yosi_UTC9 wrote: > > > Why not put "\|" in test sample? > > > > That's because "putting a caret inside the element" is a part of test. > > assert_selection() does for you. It is Selection#collapse() and > Selection#extend(). Doing this is redundant. This is not redundant. assert_selection("foo\|bar",...) is preparation for test and should not be a part of test. If so, do you think assert_selection("foo\|bar","noop","foo\|bar") intents to test Selection.collapse()? Neither it doesn't look such that and I don't think so.
	12 selection.document.execCommand("insertText", false, "bar");
	yosin_UTC9 2016/07/08 01:15:04 nit: s/"/'/g Since we make independent from "inse nit: s/"/'/g Since we make independent from "insertText" command behavior. How about checking HTMLElement.isContentEditable? Also, we may want to test in nested element case, e.g. elements in content editable w/o "contentEditable" attribute which was fixed in PS#2. BTW, what is result of Selection#extend() to attempt select partially in "user-select:all" content. It is better to have a test for "insertText" with "user-select:all" in editing/execCommand/insert_text_in_user_select_all.html. yoichio 2016/07/08 02:30:39 Done. Show quoted text On 2016/07/08 01:15:04, Yosi_UTC9 wrote: > nit: s/"/'/g > Done. Show quoted text > Since we make independent from "insertText" command behavior. > How about checking HTMLElement.isContentEditable? > Also, we may want to test in nested element case, e.g. elements in content > editable w/o "contentEditable" attribute which was fixed in PS#2. > > BTW, what is result of Selection#extend() to attempt select partially in > "user-select:all" content. > > It is better to have a test for "insertText" with "user-select:all" in > editing/execCommand/insert_text_in_user_select_all.html. This is a stable-blocker-regression that user can't type text in user-select:all and CE element. Ideally I want test to 'click' and 'enter key' other than JS selection API(thus I used eventSender). yosin_UTC9 2016/07/08 05:21:02 Stable-blocker or not isn't related to review. We Show quoted text On 2016/07/08 at 02:30:39, yoichio wrote: > On 2016/07/08 01:15:04, Yosi_UTC9 wrote: > > nit: s/"/'/g > > > Done. > > > Since we make independent from "insertText" command behavior. > > How about checking HTMLElement.isContentEditable? > > Also, we may want to test in nested element case, e.g. elements in content > > editable w/o "contentEditable" attribute which was fixed in PS#2. > > > > BTW, what is result of Selection#extend() to attempt select partially in > > "user-select:all" content. > > > > It is better to have a test for "insertText" with "user-select:all" in > > editing/execCommand/insert_text_in_user_select_all.html. > This is a stable-blocker-regression that user can't type text in user-select:all and CE element. > Ideally I want test to 'click' and 'enter key' other than JS selection API(thus I used eventSender). Stable-blocker or not isn't related to review. We should have following test case: - HTMLElement.isContentEditable - assert_selection for user-select:all - Selection#collapse - Selection#extend - Selection#setBaseAndExtend - Selection#addRange - Selection#modify - "InsertText" = No need to have Selection#collapse() - Mouse click If you expect to merge into release branch, it is OK to split C++ change and layout tests for ease of merging only C++ part. yoichio 2016/07/08 08:06:22 Done. Show quoted text On 2016/07/08 05:21:02, Yosi_UTC9 wrote: > On 2016/07/08 at 02:30:39, yoichio wrote: > > On 2016/07/08 01:15:04, Yosi_UTC9 wrote: > > > nit: s/"/'/g > > > > > Done. > > > > > Since we make independent from "insertText" command behavior. > > > How about checking HTMLElement.isContentEditable? > > > Also, we may want to test in nested element case, e.g. elements in content > > > editable w/o "contentEditable" attribute which was fixed in PS#2. > > > > > > BTW, what is result of Selection#extend() to attempt select partially in > > > "user-select:all" content. > > > > > > It is better to have a test for "insertText" with "user-select:all" in > > > editing/execCommand/insert_text_in_user_select_all.html. > > This is a stable-blocker-regression that user can't type text in > user-select:all and CE element. > > Ideally I want test to 'click' and 'enter key' other than JS selection > API(thus I used eventSender). > > Stable-blocker or not isn't related to review. > We should have following test case: > - HTMLElement.isContentEditable > - assert_selection for user-select:all > - Selection#collapse > - Selection#extend > - Selection#setBaseAndExtend > - Selection#addRange > - Selection#modify > - "InsertText" = No need to have Selection#collapse() > - Mouse click > > If you expect to merge into release branch, it is OK to split C++ change and > layout tests for ease of merging only C++ part. > > Done.
	13 },

	14 '<div contenteditable="true" id="div" style="-webkit-user-select:all">ba r\|foo</div>');

	15 assert_selection(

	16 '<div contenteditable="true">foo<div id="div" style="-webkit-user-select :all">foo</div>bar</div>',

	17 selection => {

	18 selection.collapse(selection.document.getElementById('div'), 0);

	19 selection.document.execCommand("insertText", false, "bar");

	20 },

	21 '<div contenteditable="true">foo<div id="div" style="-webkit-user-select :all">bar\|foo</div>bar</div>');

	22 },

	23 "Confirm user can insert text in -webkit-user-select:all contenteditable element ");

	24 </script>

OLD	NEW

« no previous file with comments | « no previous file | third_party/WebKit/Source/core/dom/Node.cpp » ('j') | no next file with comments »