974305 - Needs a test for WidgetPointerEvent::AssignPointerEventData() if it's possible

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I don't understand PointerEvent well, though.

If it's can be synthesized via nsIWidget, i.e., via nsIDOMWindowUtils::Send*Event(), some tests should be added into widget/tests/test_assign_event_data.html.
http://mxr.mozilla.org/mozilla-central/source/widget/tests/test_assign_event_data.html?force=1

This compares all attributes of the DOM event at dispatching and after dispatched (e.g., the event stored to global variable from an event handler and referred later).

If you don't understand how to add the test well, feel free to ask me.

Comment 1

[Maksim Lebedev]

Is it still needed?

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Maksim Lebedev from comment #1)
> Is it still needed?

Yes, if we can synthesize PointerEvent.

Comment 3

[Maksim Lebedev]

Attachment: assign_data_test_ver1.diff

+ Added two tests.

Comment and suggestion and objections are very welcome.

Comment 4

[Maksim Lebedev]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=86c3a3162398

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8594750 [details] [diff] [review]
assign_data_test_ver1.diff

>   <input id="input-text">
>   <button id="button">button</button>
>   <a id="a" href="about:blank">hyper link</a>
>   <div id="scrollable-div" style="overflow: auto; width: 30px; height: 30px;"><div id="scrolled-div" style="width: 10px; height: 10px;"></div></div>
>   <form id="form"></form>
>   <div id="animated-div"></div>
>+  <div id="pointer-div"></div>

If you can use <input> or <button> instead of adding new <div>, you should use it. However, if you need the new <div>, it's okay.

>+  { description: "PointerEvent (pointerdown)",
>+    targetID: "pointer-div", eventType: "pointerdown",
>+    dispatchEvent: function () {
>+      var elem = document.getElementById(this.targetID);
>+      var rect = elem.getBoundingClientRect();
>+      synthesizePointer(elem, rect.width/2, rect.height/2,
>+                        { type: this.eventType, button: 1, clickCount: 1, inputSource: 2, pressure: 0.25, isPrimary: true });
>+    },
>+    canRun: function () {
>+      return true;
>+    },
>+    todoMismatch: [ ],

nit: "[]", in this test.

>+  },
>+  { description: "PointerEvent (pointerup)",
>+    targetID: "pointer-div", eventType: "pointerup",
>+    dispatchEvent: function () {
>+      var elem = document.getElementById(this.targetID);
>+      var rect = elem.getBoundingClientRect();
>+      synthesizePointer(elem, rect.width/2, rect.height/2,
>+                        { type: this.eventType, button: -1, ctrlKey: true, shiftKey: true, altKey: true, isSynthesized: false });
>+    },
>+    canRun: function () {
>+      return true;
>+    },
>+    todoMismatch: [ ],

ditto.


> TEST-UNEXPECTED-ERROR | widget/tests/test_assign_event_data.html | Assertion count 34 is greater than expected range 0-32 assertions.

On windows, these tests increase assertion count. And the new assertions are not in the scope of dom/events (layout), so, please change the acceptable assertion count 0-34.

Comment 6

[Maksim Lebedev]

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #5)
> If you can use <input> or <button> instead of adding new <div>, you should
> use it. However, if you need the new <div>, it's okay.
I don't see any reasons for using <button> instead of <div>. 
Let me know, if any reason exist. I always prefer use <div>.

Comment 7

[Maksim Lebedev]

Attachment: assign_data_test_ver2.diff

+ Changed number of test assertions

Comment 8

[Maksim Lebedev]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f533679bc800

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Maksim Lebedev from comment #6)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #5)
> > If you can use <input> or <button> instead of adding new <div>, you should
> > use it. However, if you need the new <div>, it's okay.
> I don't see any reasons for using <button> instead of <div>. 
> Let me know, if any reason exist. I always prefer use <div>.

When the test is run with other tests (i.e., not alone), tests are run in small <iframe>. Therefore, the area we can synthesize mouse events is not so wide. If other tests which will be added will need new element, somebody needs to rewrite the test with opening new window. Therefore, if you don't need to create the new <div>, we can avoid such risk.

Comment 10

[Maksim Lebedev]

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #9)
> When the test is run with other tests (i.e., not alone), tests are run in
> small <iframe>. Therefore, the area we can synthesize mouse events is not so wide.
Seems reasonable.

Comment 11

[Maksim Lebedev]

Attachment: assign_data_test_ver3.diff

+ pointer div -> pointer span.
+ Add styles for elements.

Let's make the world a brighter!

Comment 12

[Maksim Lebedev]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1cc7a9cb8dec

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8595371 [details] [diff] [review]
assign_data_test_ver3.diff

rs+

Comment 14

[Maksim Lebedev]

If there are no objections, latest changes can be pushed into m-c.
Can anybody put checkin-needed flag? (I have no permissions for do that).

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/100b9eafbbfe

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/100b9eafbbfe

Comment 17

[Maksim Lebedev]

Why it did not happen?