LayoutTests/fast/events/mouse-event-buttons-attribute.html - Issue 727593003: Implement MouseEvent buttons attribute.

Side by Side Diff: LayoutTests/fast/events/mouse-event-buttons-attribute.html

Issue 727593003: Implement MouseEvent buttons attribute. (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master

Patch Set: Created 6 years, 1 month 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

« no previous file with comments | « no previous file | LayoutTests/fast/events/mouse-event-buttons-attribute-expected.txt » ('j') | LayoutTests/fast/events/mouse-event-buttons-attribute-expected.txt » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 <!DOCTYPE html>

	2 <body>
	Rick Byers 2014/11/18 20:29:19 nit: remove <body> tag - see http://www.chromium.o nit: remove <body> tag - see http://www.chromium.org/blink/coding-style/layout-test-style-guidelines#TOC-h... zino 2014/11/24 13:07:08 Done. Show quoted text On 2014/11/18 20:29:19, Rick Byers wrote: > nit: remove <body> tag - see > http://www.chromium.org/blink/coding-style/layout-test-style-guidelines#TOC-h... Done.
	3 <script src="../../resources/js-test.js"></script>

	4 <script>

	5

	6 const L = 'leftButton';

	7 const R = 'rightButton';

	8 const M = 'middleButton';

	9 const TABLE = {

	10 'leftButton': 1,

	11 'rightButton': 2,

	12 'middleButton': 4

	13 };

	14

	15 function mouseEvent()

	16 {

	17 if (window.eventSender)

	18 window.buttons = event.buttons;
	Rick Byers 2014/11/18 20:29:18 relying on the global 'event' object is discourage relying on the global 'event' object is discouraged I think (it's a little magical). Just add an argument to your function and use it instead. zino 2014/11/24 13:07:07 Done. Show quoted text On 2014/11/18 20:29:18, Rick Byers wrote: > relying on the global 'event' object is discouraged I think (it's a little > magical). Just add an argument to your function and use it instead. Done.
	19 }

	20

	21 function mouseTest(eventName, modifiers, mouseAction)

	22 {

	23 testEvent(eventName, modifiers, mouseAction, "MouseEvent");

	24 }

	25

	26 function gestureTest(eventName, modifiers, mouseAction)

	27 {

	28 testEvent(eventName, modifiers, mouseAction, "GestureEvent");

	29 }

	30

	31 function wheelTest(eventName, modifiers, mouseAction)

	32 {

	33 testEvent(eventName, modifiers, mouseAction, "WheelEvent");

	34 }

	35
	Rick Byers 2014/11/18 20:29:19 add 'var buttons' to make it clear you're creating add 'var buttons' to make it clear you're creating a new global variable. zino 2014/11/24 13:07:08 Done. Show quoted text On 2014/11/18 20:29:19, Rick Byers wrote: > add 'var buttons' to make it clear you're creating a new global variable. Done.
	36 function testEvent(eventName, modifiers, mouseAction, description)

	37 {

	38 if (!window.eventSender)

	39 return;

	40

	41 var div = document.createElement('div');

	42 div.style.width = '100px';

	43 div.style.height = '100px';

	44 document.body.insertBefore(div, document.body.firstChild);
	Rick Byers 2014/11/18 20:29:18 Is there a reason you want to create a new div for Is there a reason you want to create a new div for each test? It would be simpler to just define a single target div in HTML. zino 2014/11/24 13:07:07 Done. I'm not sure but shouldXXXX series in test. Show quoted text On 2014/11/18 20:29:18, Rick Byers wrote: > Is there a reason you want to create a new div for each test? It would be > simpler to just define a single target div in HTML. Done. I'm not sure but shouldXXXX series in test.js seems to have some bug when using eventSender or event listener. Rick Byers 2014/11/25 17:44:27 Perhaps what you were seeing was that the console Show quoted text On 2014/11/24 13:07:07, zino wrote: > On 2014/11/18 20:29:18, Rick Byers wrote: > > Is there a reason you want to create a new div for each test? It would be > > simpler to just define a single target div in HTML. > > Done. > > I'm not sure but shouldXXXX series in test.js seems to have some bug when using > eventSender or event listener. Perhaps what you were seeing was that the console by default gets created ABOVE your test div, so as shouldBe is generating output, the location of your div is changing so that it's no longer under the co-ordinates you've created. I've dealt with this in the past by explicitly adding a <div id=console></div> tag below my test element, or making my test element position:absolute over at the right side of the document. If you do either of those, then you should be able to combine your two loops into one again. zino 2014/11/28 12:29:28 Done. Show quoted text On 2014/11/25 17:44:27, Rick Byers wrote: > On 2014/11/24 13:07:07, zino wrote: > > On 2014/11/18 20:29:18, Rick Byers wrote: > > > Is there a reason you want to create a new div for each test? It would be > > > simpler to just define a single target div in HTML. > > > > Done. > > > > I'm not sure but shouldXXXX series in test.js seems to have some bug when > using > > eventSender or event listener. > > Perhaps what you were seeing was that the console by default gets created ABOVE > your test div, so as shouldBe is generating output, the location of your div is > changing so that it's no longer under the co-ordinates you've created. I've > dealt with this in the past by explicitly adding a <div id=console></div> tag > below my test element, or making my test element position:absolute over at the > right side of the document. > > If you do either of those, then you should be able to combine your two loops > into one again. Done.
	45
	Rick Byers 2014/11/18 20:29:19 You should probably verify that your event handler You should probably verify that your event handler is getting called at all (especially since you need to add a case where the expected value is 0 - see below). Perhaps set buttons to some illegal value (eg. 'null') right here? Then the fact that it's a number at all means the event handler got invoked (and also there's also no reason to reset it to 0 below). zino 2014/11/24 13:07:08 Done. Show quoted text On 2014/11/18 20:29:19, Rick Byers wrote: > You should probably verify that your event handler is getting called at all > (especially since you need to add a case where the expected value is 0 - see > below). Perhaps set buttons to some illegal value (eg. 'null') right here? > Then the fact that it's a number at all means the event handler got invoked (and > also there's also no reason to reset it to 0 below). Done.
	46 div.addEventListener(eventName, mouseEvent, false);

	47 mouseAction(modifiers);

	48 div.removeEventListener(eventName, mouseEvent, false);

	49

	50 var expectedValue = 0;

	51 for (var i = 0; i < modifiers.length; i++)

	52 expectedValue \|= TABLE[modifiers[i]]

	53

	54 debug(description + '::' + eventName + ' test [' + modifiers + ']');

	55 shouldBe('buttons', expectedValue.toString());
	Rick Byers 2014/11/18 20:29:19 why toString? Maybe use shouldBeEqualToNumber ins why toString? Maybe use shouldBeEqualToNumber instead? zino 2014/11/24 13:07:08 Done. Oh, I didn't know that. Show quoted text On 2014/11/18 20:29:19, Rick Byers wrote: > why toString? Maybe use shouldBeEqualToNumber instead? Done. Oh, I didn't know that.
	56 debug('');

	57

	58 window.buttons = 0;

	59 eventSender.mouseMoveTo(-1, -1);

	60 eventSender.mouseUp();

	61 }

	62

	63 mouseTest('click', [L, R], function(modifiers) {

	64 eventSender.mouseMoveTo(50, 50, modifiers);

	65 eventSender.mouseDown(0, modifiers);

	66 eventSender.mouseUp(0, modifiers);

	67 });

	68

	69 mouseTest('dblclick', [L, M], function(modifiers) {

	70 eventSender.mouseMoveTo(50, 50);

	71 eventSender.mouseDown(0, modifiers);

	72 eventSender.mouseUp(0, modifiers);

	73 eventSender.mouseDown(0, modifiers);

	74 eventSender.mouseUp(0, modifiers);

	75 });

	76

	77 mouseTest('mousedown', [L, M, R], function(modifiers) {

	78 eventSender.mouseMoveTo(50, 50, modifiers);

	79 eventSender.mouseDown(0, modifiers);

	80 eventSender.mouseUp(0, modifiers);

	81 });

	82

	83 mouseTest('mouseup', [R], function(modifiers) {

	84 eventSender.mouseMoveTo(50, 50, modifiers);

	85 eventSender.mouseDown(0, modifiers);

	86 eventSender.mouseUp(0, modifiers);

	87 });

	88

	89 mouseTest('mousemove', [L], function(modifiers) {
	Rick Byers 2014/11/18 20:29:19 you should have one test where there are no modifi you should have one test where there are no modifiers - eg. for a mousemove event (to verify you get the 0 value). zino 2014/11/24 13:07:08 Done. Show quoted text On 2014/11/18 20:29:19, Rick Byers wrote: > you should have one test where there are no modifiers - eg. for a mousemove > event (to verify you get the 0 value). Done.
	90 eventSender.mouseMoveTo(-1, -1, modifiers);

	91 eventSender.mouseMoveTo(50, 50, modifiers);

	92 });

	93

	94 mouseTest('mouseenter', [R, M], function(modifiers) {

	95 eventSender.mouseMoveTo(-1, -1, modifiers);

	96 eventSender.mouseMoveTo(50, 50, modifiers);

	97 });

	98

	99 mouseTest('mouseleave', [L, R], function(modifiers) {

	100 eventSender.mouseMoveTo(50, 50, modifiers);

	101 eventSender.mouseMoveTo(-1, -1, modifiers);

	102 });

	103

	104 mouseTest('mouseover', [L, M], function(modifiers) {

	105 eventSender.mouseMoveTo(-1, -1, modifiers);

	106 eventSender.mouseMoveTo(50, 50, modifiers);

	107 });

	108

	109 mouseTest('mouseout', [L], function(modifiers) {

	110 eventSender.mouseMoveTo(50, 50, modifiers);

	111 eventSender.mouseMoveTo(-1, -1, modifiers);

	112 });

	113

	114 wheelTest('mousewheel', [L, R], function(modifiers) {

	115 eventSender.mouseMoveTo(50, 50, modifiers);

	116 eventSender.mouseScrollBy(0, 120, false, true, modifiers);

	117 });

	118

	119 gestureTest('mousedown', [L], function() {

	120 eventSender.gestureTap(50, 50);

	121 });

	122

	123 gestureTest('mouseup', [L], function() {

	124 eventSender.gestureTap(50, 50);

	125 });

	126

	127 gestureTest('click', [L], function() {

	128 eventSender.gestureTap(50, 50);

	129 });

	130

	131 gestureTest('dblclick', [L], function() {

	132 eventSender.gestureTap(50, 50, 2);

	133 });

	134

	135 gestureTest('mousemove', [L], function() {

	136 eventSender.gestureTap(50, 50);

	137 });

	138

	139 </script>

	140 </body>

OLD	NEW