Closed Bug 985511 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: DOM: Events, defect)

x86_64
Windows 8
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: alessarik, Assigned: alessarik)

References

(Blocks 1 open bug)

Details

(Keywords: testcase)

Attachments

(1 file, 8 obsolete files)

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
Component: Untriaged → DOM: Events
Keywords: testcase
Product: Firefox → Core
Attached patch pen_touch_leave_ver1.diff (obsolete) — Splinter Review
Any comments and suggestion are welcome.
Attachment #8393562 - Flags: review?(bugs)
Attachment #8393562 - Flags: feedback?(oleg.romashin)
Attachment #8393562 - Flags: feedback?(nicklebedev37)
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 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 on attachment 8393562 [details] [diff] [review]
pen_touch_leave_ver1.diff

Yeah, tryserver needed here. And tests.
Attachment #8393562 - Flags: review?(bugs)
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?
Attachment #8393562 - Flags: feedback?(oleg.romashin)
Attached patch pen_touch_leave_ver2.diff (obsolete) — Splinter Review
+ 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
Attachment #8393562 - Attachment is obsolete: true
Attachment #8393562 - Flags: feedback?(nicklebedev37)
Attachment #8406184 - Flags: review?(mounir)
Attachment #8406184 - Flags: review?(bugs)
Attachment #8406184 - Flags: feedback?(oleg.romashin)
Attachment #8406184 - Flags: feedback?(nicklebedev37)
Polyfill Test System and simple converter to Mochitest System
Attachment #8406187 - Flags: review?(mounir)
Attachment #8406187 - Flags: review?(bugs)
Attachment #8406187 - Flags: feedback?(oleg.romashin)
Attachment #8406187 - Flags: feedback?(nicklebedev37)
Simple test for bug
Attachment #8406191 - Flags: review?(mounir)
Attachment #8406191 - Flags: review?(bugs)
Attachment #8406191 - Flags: feedback?(oleg.romashin)
Attachment #8406191 - Flags: feedback?(nicklebedev37)
Attachment #8406191 - Flags: review?(mounir)
Attachment #8406191 - Flags: feedback?(nicklebedev37)
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.
Attachment #8406191 - Flags: review?(bugs) → review-
Comment on attachment 8406184 [details] [diff] [review]
pen_touch_leave_ver2.diff

># HG changeset patch
># Parent 219b18c247334a11ee3f4005dc20efc95ce5876a
># User Maksim Lebedev <alessarik@gmail.com>
>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
Attachment #8406184 - Flags: review?(mounir)
Attachment #8406184 - Flags: review?(bugs)
Attachment #8406184 - Flags: review+
Attachment #8406184 - Flags: feedback?(nicklebedev37)
Attachment #8406187 - Flags: review?(mounir)
Attachment #8406187 - Flags: feedback?(nicklebedev37)
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.
Attachment #8406187 - Flags: review?(bugs)
+ changes according with comments
Attachment #8406184 - Attachment is obsolete: true
Attachment #8406184 - Flags: feedback?(oleg.romashin)
Attachment #8407377 - Flags: review?(bugs)
Attachment #8407377 - Flags: feedback?(oleg.romashin)
(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.
Attachment #8406187 - Attachment is obsolete: true
Attachment #8406187 - Flags: feedback?(oleg.romashin)
Attachment #8407587 - Flags: review?(jst)
Attachment #8407587 - Flags: review?(bugs)
Attachment #8407587 - Flags: feedback?(oleg.romashin)
Attached patch pointerEvents_tests_ver2.diff (obsolete) — Splinter Review
+ changes in mochitest.ini
+ add one test
Attachment #8406191 - Attachment is obsolete: true
Attachment #8406191 - Flags: feedback?(oleg.romashin)
Attachment #8407611 - Flags: review?(jst)
Attachment #8407611 - Flags: review?(bugs)
Attachment #8407611 - Flags: feedback?(oleg.romashin)
Attachment #8407611 - Flags: feedback?(nicklebedev37)
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)
Attachment #8407587 - Flags: feedback?(oleg.romashin)
Comment on attachment 8407587 [details] [diff] [review]
pointerEventsTestSystem_ver2.diff

One review is enough.
Attachment #8407587 - Flags: review?(jst)
Attachment #8407611 - Flags: review?(jst)
Attachment #8407611 - Flags: feedback?(oleg.romashin)
Attached patch off_tests_ver3.diff (obsolete) — Splinter Review
+ official tests for pointer events
Attachment #8407587 - Attachment is obsolete: true
Attachment #8407611 - Attachment is obsolete: true
Attachment #8407587 - Flags: review?(bugs)
Attachment #8407611 - Flags: review?(bugs)
Attachment #8407611 - Flags: feedback?(nicklebedev37)
Attachment #8408794 - Flags: feedback?(oleg.romashin)
Attachment #8408794 - Flags: feedback?(nicklebedev37)
Attachment #8408794 - Flags: feedback?(bugs)
(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);
>}
Attached patch off_tests_ver5.diff (obsolete) — Splinter Review
+ update some of tests
Attachment #8408794 - Attachment is obsolete: true
Attachment #8408794 - Flags: feedback?(oleg.romashin)
Attachment #8408794 - Flags: feedback?(nicklebedev37)
Attachment #8408794 - Flags: feedback?(bugs)
Attachment #8410211 - Flags: feedback?(oleg.romashin)
Attachment #8410211 - Flags: feedback?(nicklebedev37)
Attachment #8410211 - Flags: feedback?(bugs)
Attachment #8407377 - Flags: review?(bugs) → review+
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/ )
Attachment #8410211 - Flags: feedback?(bugs)
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 ?
So from where are the tests coming? Has someone reviewed them already?
Assignee: nobody → alessarik
Blocks: pointerevent
Attachment #8410211 - Attachment is obsolete: true
Attachment #8410211 - Flags: feedback?(oleg.romashin)
Attachment #8410211 - Flags: feedback?(nicklebedev37)
I will try to sinchronise all tests with reviewed copies from official w3c repository.
Now all tests are situated in bug 1000870.
Keywords: checkin-needed
Version: 26 Branch → Trunk
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
https://hg.mozilla.org/mozilla-central/rev/bbad533aa83e
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Target Milestone: mozilla32 → mozilla31
Blocks: 1018113
You need to log in before you can comment on or make changes to this bug.