985511 - When a pointing device that supports hover leaves the range of the digitizer while over an element, the pointerleave event must be dispatched

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[Maksim Lebedev]

User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0 (Beta/Release)
Build ID: 20131205075310

Steps to reproduce:

Test 9.1.1 https://github.com/InternetExplorer/web-platform-tests/blob/submission/Microsoft/PointerEvents/pointerevents/pointerevent_pointerLeave_pen.html


Actual results:

If I use pen or touch I cannot receive pointerleave event when I lift up pen or finger.


Expected results:

Spec: User agents MUST also fire a pointer event named pointerleave when a pen stylus leaves hover range detectable by the digitizer

Comment 1

[Maksim Lebedev]

Attachment: pen_touch_leave_ver1.diff

Any comments and suggestion are welcome.

Comment 2

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

Comment on attachment 8393562 [details] [diff] [review]
pen_touch_leave_ver1.diff

Tests please. We really need to start adding more tests for pointer event stuff.

Comment 3

[Oleg Romashin (MS)]

Comment on attachment 8393562 [details] [diff] [review]
pen_touch_leave_ver1.diff

Could you create try build with just this patch? not sure but we may need to fix some tests as well or create new.

Comment 4

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

Comment on attachment 8393562 [details] [diff] [review]
pen_touch_leave_ver1.diff

Yeah, tryserver needed here. And tests.

Comment 5

[Oleg Romashin (MS)]

Comment on attachment 8393562 [details] [diff] [review]
pen_touch_leave_ver1.diff

>+      if (targetElement) {
>+        OverOutElementsWrapper* helper = GetWrapperByEventID(aMouseEvent);
>+        helper->mLastOverElement = targetElement;

I think helper need null check here?

Comment 6

[Maksim Lebedev]

Attachment: pen_touch_leave_ver2.diff

+ changes related with last version of source code

(In reply to Oleg Romashin (MS) from comment #5)
> >+      if (targetElement) {
> >+        OverOutElementsWrapper* helper = GetWrapperByEventID(aMouseEvent);
> >+        helper->mLastOverElement = targetElement;
> I think helper need null check here?
1. All code have no check helper
2. GetWrapperById in all cases returns valid object

Comment 7

[Maksim Lebedev]

Attachment: pointerEventsTestSystem_ver1.diff

Polyfill Test System and simple converter to Mochitest System

Comment 8

[Maksim Lebedev]

Attachment: test_pointerevent_pointerLeave_pen_ver1.diff

Simple test for bug

Comment 9

[Maksim Lebedev]

https://tbpl.mozilla.org/?tree=Try&rev=725b018537aa

Comment 10

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

Comment on attachment 8406191 [details] [diff] [review]
test_pointerevent_pointerLeave_pen_ver1.diff


>+[DEFAULT]
>+skip-if = toolkit == 'android' #CRASH_DUMP, RANDOM
What crash dump?


Should not push stuff which crashes randomly.

Comment 11

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

Comment on attachment 8406184 [details] [diff] [review]
pen_touch_leave_ver2.diff

># HG changeset patch
># Parent 219b18c247334a11ee3f4005dc20efc95ce5876a
># User Maksim Lebedev <[e-mail]>
>Bug 985511 - When a pointing device that supports hover leaves the range of the digitizer while over an element, the pointerleave event must be dispatched
>
>diff --git a/dom/events/EventStateManager.cpp b/dom/events/EventStateManager.cpp
>--- a/dom/events/EventStateManager.cpp

>+      if (targetElement) {
>+        OverOutElementsWrapper* helper = GetWrapperByEventID(aMouseEvent);
>+        helper->mLastOverElement = targetElement;
null check helper

Comment 12

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

Comment on attachment 8406187 [details] [diff] [review]
pointerEventsTestSystem_ver1.diff

We have testharness already in
http://mxr.mozilla.org/mozilla-central/source/dom/imptests/testharness.js

So could you update the patch to use that.

Comment 13

[Maksim Lebedev]

Attachment: pen_touch_leave_ver3.diff

+ changes according with comments

Comment 14

[Maksim Lebedev]

Attachment: pointerEventsTestSystem_ver2.diff

(In reply to Olli Pettay [:smaug] from comment #12)
> We have testharness already in
> http://mxr.mozilla.org/mozilla-central/source/dom/imptests/testharness.js
> So could you update the patch to use that.
Thanks. I will use it.

Comment 15

[Maksim Lebedev]

Attachment: pointerEvents_tests_ver2.diff

+ changes in mochitest.ini
+ add one test

Comment 16

[Oleg Romashin (MS)]

Comment on attachment 8407587 [details] [diff] [review]
pointerEventsTestSystem_ver2.diff

Did you use SpecialPowers to enable Pointer Events in your tests?

Could you send try build with patches applied and remove "Implement pointer events. Interface enabler", 
Tests should enable that preference inside tests Initialize function using Special powers (see other similar PointerEvent tests)

Comment 17

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

Comment on attachment 8407587 [details] [diff] [review]
pointerEventsTestSystem_ver2.diff

One review is enough.

Comment 18

[Maksim Lebedev]

Attachment: off_tests_ver3.diff

+ official tests for pointer events

Comment 19

[Maksim Lebedev]

(In reply to Oleg Romashin (MS) from comment #16)
> Did you use SpecialPowers to enable Pointer Events in your tests?
Yes. Please see supportMochiTest.js
>addEventListener("load", function(event) {preExecute();}, false);
>function preExecute() {
> SpecialPowers.pushPrefEnv({"set": [["dom.w3c_pointer_events.enabled", true]]}, translateSystem);
>}

Comment 20

[Maksim Lebedev]

Attachment: off_tests_ver5.diff

+ update some of tests

Comment 21

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

Comment on attachment 8410211 [details] [diff] [review]
off_tests_ver5.diff

>+[test_pointerevent_button_attribute_mouse.html]
>+[test_pointerevent_capture.html]
>+[test_pointerevent_change-touch-action-onpointerdown_touch.html]
>+[test_pointerevent_gotpointercapture_before_first_pointerevent.html]
>+[test_pointerevent_lostpointercapture_is_first.html]
>+[test_pointerevent_pointercancel_touch.html]
>+[test_pointerevent_pointerdown.html]
>+[test_pointerevent_pointerenter.html]
>+[test_pointerevent_pointerenter_does_not_bubble.html]
>+[test_pointerevent_pointerenter_nohover.html]
>+[test_pointerevent_pointerleave_descendant_over.html]
>+[test_pointerevent_pointerleave_descendants.html]
>+[test_pointerevent_pointerleave_does_not_bubble.html]
>+[test_pointerevent_pointerleave_mouse.html]
>+[test_pointerevent_pointerleave_pen.html]
>+[test_pointerevent_pointerleave_touch.html]
>+[test_pointerevent_pointermove_isprimary_same_as_pointerdown.html]
>+[test_pointerevent_pointermove_pointertype.html]
>+[test_pointerevent_pointerout.html]
>+[test_pointerevent_pointerout_received_once.html]
>+[test_pointerevent_pointerover.html]
>+[test_pointerevent_pointertype_mouse.html]
>+[test_pointerevent_pointertype_pen.html]
>+[test_pointerevent_pointertype_touch.html]
>+[test_pointerevent_pointerup.html]
>+[test_pointerevent_releasecapture_onpointercancel.html]
>+[test_pointerevent_releasecapture_onpointerup.html]
>+[test_pointerevent_releasepointercapture_events_to_original_target.html]
>+[test_pointerevent_releasepointercapture_invalid_pointerid.html]
>+[test_pointerevent_setpointercapture_inactive_button.html]
>+[test_pointerevent_setpointercapture_invalid_pointerid.html]
>+[test_pointerevent_setpointercapture_relatedtarget.html]
>+#[test_pointerevent_touch-action-auto-css_touch.html]
>+[test_pointerevent_touch-action-illegal.html]
>+#[test_pointerevent_touch-action-inherit_child-auto-child-none_touch.html]
>+#[test_pointerevent_touch-action-inherit_child-none_touch.html]
>+#[test_pointerevent_touch-action-inherit_child-pan-x-child-pan-x_touch.html]
>+#[test_pointerevent_touch-action-inherit_child-pan-x-child-pan-y_touch.html]
>+#[test_pointerevent_touch-action-inherit_highest-parent-none_touch.html]
>+#[test_pointerevent_touch-action-inherit_parent-none_touch.html]
>+#[test_pointerevent_touch-action-keyboard.html]
>+[test_pointerevent_touch-action-mouse.html]
>+#[test_pointerevent_touch-action-none-css_touch.html]
>+#[test_pointerevent_touch-action-pan-x-css_touch.html]
>+#[test_pointerevent_touch-action-pan-x-pan-y-pan-y_touch.html]
>+#[test_pointerevent_touch-action-pan-x-pan-y_touch.html]
>+#[test_pointerevent_touch-action-pan-y-css_touch.html]

So where are all these tests coming from? From w3c repository?
Are they reviewed tests or still waiting for review?

Clearing review request until that is clarified.
(I think some tests are still waiting for review https://github.com/w3c/web-platform-tests/pull/324/ )

Comment 22

[Maksim Lebedev]

Tests are coming from w3c repositoriy (with minimal changes), but from another branch.
In this case maybe the correct way is moving tests for new bug and commit pen_touch_leave_ver3.diff ?

Comment 23

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

So from where are the tests coming? Has someone reviewed them already?

Comment 24

[Maksim Lebedev]

https://tbpl.mozilla.org/?tree=Try&rev=6767f2551051

Comment 25

[Maksim Lebedev]

I will try to sinchronise all tests with reviewed copies from official w3c repository.
Now all tests are situated in bug 1000870.

Comment 26

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bbad533aa83e

Comment 27

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bbad533aa83e