LayoutTests/fast/forms/setrangetext-within-events.html - Issue 507533002: Fix to keep the selection of the text field in input element after setSelectionRange is called by d…

Side by Side Diff: LayoutTests/fast/forms/setrangetext-within-events.html

Issue 507533002: Fix to keep the selection of the text field in input element after setSelectionRange is called by d… (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master

Patch Set: Created 6 years, 3 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/js-test.js"></script>

	3 <script>

	4 window.onload = function() {
	Rick Byers 2014/08/28 18:16:41 please add a description() call please add a description() call Miyoung Shin(g) 2014/08/29 14:18:52 OK, I will Show quoted text On 2014/08/28 18:16:41, Rick Byers wrote: > please add a description() call OK, I will
	5 if (window.eventSender) {

	6 doSetSelectionRange('focus');

	7 doSetSelectionRange('mousedown');

	8 doSetSelectionRange('mouseup');

	9 doSetSelectionRange('click');

	10 }

	11 }

	12

	13 function doSetSelectionRange(event) {

	14 debug(event + ' :');

	15 var textfield = document.getElementById('textfield');

	16 var tx = textfield.offsetLeft + 4;

	17 var ty = textfield.offsetTop + 4;

	18

	19 textfield.addEventListener(event, setSelectionRange);

	20 eventSender.mouseMoveTo(tx, ty);

	21 if (event == 'mousedown') {
	Rick Byers 2014/08/28 18:16:41 is this behavior of clearing for changes in moused is this behavior of clearing for changes in mousedown but not in the other 3 events consistent with other browsers? That seems a little odd to me - perhaps selection changes during mousedown also shouldn't be overwritten? Miyoung Shin(g) 2014/08/29 14:18:52 This is intended to work like it. I referred tota Show quoted text On 2014/08/28 18:16:41, Rick Byers wrote: > is this behavior of clearing for changes in mousedown but not in the other 3 > events consistent with other browsers? That seems a little odd to me - perhaps > selection changes during mousedown also shouldn't be overwritten? This is intended to work like it. I referred totally to FF's behavior. FF doesn't keep the selection by setSelectionRange() in case of mousedown event. And I agree with their behavior because we can't handle the selection by user if we keep the selection in case of mousedown. Rick Byers 2014/08/29 15:48:05 Sounds reasonable, thanks. Show quoted text On 2014/08/29 14:18:52, Miyoung Shin(g) wrote: > On 2014/08/28 18:16:41, Rick Byers wrote: > > is this behavior of clearing for changes in mousedown but not in the other 3 > > events consistent with other browsers? That seems a little odd to me - > perhaps > > selection changes during mousedown also shouldn't be overwritten? > This is intended to work like it. > I referred totally to FF's behavior. > FF doesn't keep the selection by setSelectionRange() in case of mousedown event. > And I agree with their behavior because we can't handle the selection by user if > we keep the selection in case of mousedown. Sounds reasonable, thanks.
	22 eventSender.mouseDown();

	23 eventSender.mouseUp();

	24 shouldBe('textfield.selectionStart', '0');

	25 shouldBe('textfield.selectionEnd', '0');

	26 } else {

	27 eventSender.mouseDown();

	28 eventSender.mouseUp();
	Rick Byers 2014/08/28 18:16:41 The first three lines here are duplicated with abo The first three lines here are duplicated with above - make only the last shouldBe conditional. Miyoung Shin(g) 2014/08/29 14:18:52 OK, I will Show quoted text On 2014/08/28 18:16:41, Rick Byers wrote: > The first three lines here are duplicated with above - make only the last > shouldBe conditional. OK, I will
	29 shouldBe('textfield.selectionStart', '0');

	30 shouldBe('textfield.selectionEnd', '5');

	31 }

	32 eventSender.mouseMoveTo(tx, ty + 100);
	Rick Byers 2014/08/28 18:16:41 Is this click here to clear the selection? If so Is this click here to clear the selection? If so please verify that it actually got cleared. Miyoung Shin(g) 2014/08/29 14:18:52 OK, I will Show quoted text On 2014/08/28 18:16:41, Rick Byers wrote: > Is this click here to clear the selection? If so please verify that it actually > got cleared. OK, I will
	33 eventSender.mouseDown();

	34 eventSender.mouseUp();

	35 textfield.removeEventListener(event, setSelectionRange);

	36 }

	37

	38 function setSelectionRange(e) {

	39 var textfield = document.getElementById('textfield');
	Rick Byers 2014/08/28 18:16:41 perhaps this function should output a log entry to perhaps this function should output a log entry too, just to confirm that the event handler actually got invoked at all (and we're not just keeping the selection from the last time around)? Miyoung Shin(g) 2014/08/29 14:18:52 If I try to output here, we may lost focus of inpu Show quoted text On 2014/08/28 18:16:41, Rick Byers wrote: > perhaps this function should output a log entry too, just to confirm that the > event handler actually got invoked at all (and we're not just keeping the > selection from the last time around)? If I try to output here, we may lost focus of input element, then the selection is cleared. For preventing flaky issue, I don't want to add here any log Rick Byers 2014/08/29 15:48:05 Ok, even better then - how about resetting the sel Show quoted text On 2014/08/29 14:18:52, Miyoung Shin(g) wrote: > On 2014/08/28 18:16:41, Rick Byers wrote: > > perhaps this function should output a log entry too, just to confirm that the > > event handler actually got invoked at all (and we're not just keeping the > > selection from the last time around)? > > If I try to output here, we may lost focus of input element, then the selection > is cleared. > For preventing flaky issue, I don't want to add here any log Ok, even better then - how about resetting the selection state at the start of each test case - eg. textfield.setSelectionRange(0,1) so that we can tell from the output that the selection was actually changed from it's dummy initial state?
	40 textfield.setSelectionRange(0, textfield.value.length);

	41 }

	42 </script>

	43 <input type="text" value="value" id="textfield"></input>

OLD	NEW

« no previous file with comments | « no previous file | LayoutTests/fast/forms/setrangetext-within-events-expected.txt » ('j') | Source/core/page/EventHandler.cpp » ('J')