The API is still very my in flux, but we should start implementing it soon.

cc: Matt Brubeck for input.

For what input ? :) We should implement pointer events, and the spec is getting more stable.

Yes, I agree. We are about to publish a second Last Call Working Draft to address comments from our first LCWD; the time to start implementing is now (especially if we want any feedback we discover during implementation to influence the final Recommendation).

Here is the initial implementation for Pointer Events. Current WIP is partial implementation which has: 1) Definition of PointerEvent type in content/dom/webidl/widget derived from MouseEvent 2) Basic Implementation of Content EventManager tricks for out/over enter/leave part. Mainly copied from MouseEvent implementation, and currently not merged with Mouse functions Ideally need to merge NotifyMouse/Out/Over and Enter/Leave dispatchers and share them between Mouse and Pointer events 3) May/HavePointerEvents implementation for content/dom 4) Declaration for pointerEnabled maxTouchPoints attributes without real implementation 5) setPointerCapture - without implementation (need to connect/share it to mouse event setCapture API) 6) Dummy stub for TabChild stuff... TBD 7) Quick wrapper in nsView which convert Mouse events and TouchEvents into Pointer events when page have any pointer listeners. 8) Basic declaration of touch-action, need some help in order to implement it properly, or point to existing Css property with similar functionality 9) Basic unit test for pointer events All changes attached in one blob, but I will split logical parts into separate patches to make reviewing process easier. Need some advice on license headers format... (not sure what is the right way to attach 3-rd party copyright contributions) Basically it would be nice to overlook on patch and figure out if we can do better integration between Mouse/Pointer events. Any other comments are welcome.

Will pointer events improve the responsiveness of a pen in FF? I've tried drawing on HTML5 canvas on a Windows 8 tablet and a Galaxy Note 10.1 both with a Wacom digitizer and the responsiveness is bad compared to IE10 on Windows 8 and native apps on the Galaxy Note.

Pointer events will have a bit more cheap processing code due to no TouchPoints list... but I believe in your case that problem related to something else, possibly Graphics...

I don't think its a Graphics issue because on Win 8, for example, IE10 works great. You can see the bug details here including a test page. https://bugzilla.mozilla.org/show_bug.cgi?id=827247

Comment on attachment 763013 [details] [diff] [review] WIP Implementation for Pointer events >+++ b/content/base/src/nsTreeSanitizer.cpp >@@ -1,9 +1,10 @@ > /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ >+/* Portions Copyright (c) Microsoft Open Technologies, Inc. All Rights Reserved. */ > /* vim: set sw=2 ts=2 et tw=80: */ > /* This Source Code Form is subject to the terms of the Mozilla Public > * License, v. 2.0. If a copy of the MPL was not distributed with this > * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ > > #include "mozilla/Util.h" > > #include "nsTreeSanitizer.h" >@@ -496,16 +497,17 @@ nsIAtom** const kAttributesSVG[] = { > &nsGkAtoms::overflow, // overflow > // panose-1 > &nsGkAtoms::path, // path > &nsGkAtoms::pathLength, // pathLength > &nsGkAtoms::patternContentUnits, // patternContentUnits > &nsGkAtoms::patternTransform, // patternTransform > &nsGkAtoms::patternUnits, // patternUnits > &nsGkAtoms::pointer_events, // pointer-events XXX is this safe? >+ &nsGkAtoms::touch_action, // touch action Why we need this? >+nsDOMPointerEvent::nsDOMPointerEvent(mozilla::dom::EventTarget* aOwner, >+ nsPresContext* aPresContext, >+ nsPointerEvent* aEvent) >+ : nsDOMMouseEvent(aOwner, aPresContext, aEvent ? aEvent : new nsPointerEvent(false, 0, nullptr)) >+{ >+ NS_ASSERTION(mEvent->eventStructType == NS_POINTER_EVENT, "event type mismatch NS_POINTER_EVENT"); >+ >+ if (aEvent) { >+ mEventIsInternal = false; >+ } else { >+ mEventIsInternal = true; >+ mEvent->time = PR_Now(); >+ mEvent->refPoint.x = mEvent->refPoint.y = 0; >+ static_cast<nsMouseEvent*>(mEvent)->inputSource = nsIDOMMouseEvent::MOZ_SOURCE_UNKNOWN; >+ } >+ SetIsDOMBinding(); All events use now Webidl bindings so no need for this. >+already_AddRefed<nsDOMPointerEvent> >+nsDOMPointerEvent::Constructor(const mozilla::dom::GlobalObject& aGlobal, >+ const nsAString& aType, >+ const mozilla::dom::PointerEventInit& aParam, >+ mozilla::ErrorResult& aRv) align params >+ConvertStringToPointerType(const nsAString& aPointerTypeArg) >+{ >+ if (aPointerTypeArg.EqualsLiteral("mouse")) >+ return nsIDOMMouseEvent::MOZ_SOURCE_MOUSE; >+ if (aPointerTypeArg.EqualsLiteral("pen")) >+ return nsIDOMMouseEvent::MOZ_SOURCE_PEN; >+ if (aPointerTypeArg.EqualsLiteral("touch")) >+ return nsIDOMMouseEvent::MOZ_SOURCE_TOUCH; >+ >+ return nsIDOMMouseEvent::MOZ_SOURCE_UNKNOWN; >+} Use {} with if. >+nsDOMPointerEvent::InitPointerEvent(const nsAString& typeArg, >+ bool canBubbleArg, >+ bool cancelableArg, >+ nsIDOMWindow* viewArg, >+ int32_t detailArg, >+ int32_t screenXArg, >+ int32_t screenYArg, >+ int32_t clientXArg, >+ int32_t clientYArg, >+ bool ctrlKeyArg, >+ bool altKeyArg, >+ bool shiftKeyArg, >+ bool metaKeyArg, >+ uint16_t buttonArg, >+ nsIDOMEventTarget* relatedTargetArg, >+ int32_t pointerIdArg, >+ int32_t widthArg, >+ int32_t heightArg, >+ float pressureArg, >+ int32_t tiltXArg, >+ int32_t tiltYArg, >+ const nsAString& pointerTypeArg, >+ bool isPrimaryArg) parameter names should be aParamName >+ void InitPointerEvent(const nsAString& typeArg, >+ bool canBubbleArg, >+ bool cancelableArg, >+ nsIDOMWindow* viewArg, >+ int32_t detailArg, >+ int32_t screenXArg, >+ int32_t screenYArg, >+ int32_t clientXArg, >+ int32_t clientYArg, >+ bool ctrlKeyArg, >+ bool altKeyArg, >+ bool shiftKeyArg, >+ bool metaKeyArg, >+ uint16_t buttonArg, >+ mozilla::dom::EventTarget* relatedTargetArg, >+ int32_t pointerIdArg, >+ int32_t widthArg, >+ int32_t heightArg, >+ float pressureArg, >+ int32_t tiltXArg, >+ int32_t tiltYArg, >+ const nsAString& pointerTypeArg, >+ bool isPrimaryArg, >+ mozilla::ErrorResult& aRv) Please use the form aParamName for parameter >+ if (aEventType.LowerCaseEqualsLiteral("pointerevent")) >+ return NS_NewDOMPointerEvent(aDOMEvent, aOwner, aPresContext, nullptr); We don't want this for new events types. New types should be creatable only using new EventType(...) >+ } else if (aTypeAtom == nsGkAtoms::onpointerdown || >+ aTypeAtom == nsGkAtoms::onpointerup || >+ aTypeAtom == nsGkAtoms::onpointermove || >+ aTypeAtom == nsGkAtoms::onpointercancel || >+ aTypeAtom == nsGkAtoms::onpointerover || >+ aTypeAtom == nsGkAtoms::onpointerout || >+ aTypeAtom == nsGkAtoms::onpointerenter || >+ aTypeAtom == nsGkAtoms::onpointerleave || >+ aTypeAtom == nsGkAtoms::ongotpointercapture || >+ aTypeAtom == nsGkAtoms::onlostpointercapture) { Should be faster to compare using aType, similar to mutation events >+ mMayHavePointerEventListener = true; >+ nsPIDOMWindow* window = GetInnerWindowForTarget(); >+ // we don't want touchevent listeners added by scrollbars to flip this flag pointerevents >@@ -524,18 +527,20 @@ protected: > > uint32_t mMayHavePaintEventListener : 1; > uint32_t mMayHaveMutationListeners : 1; > uint32_t mMayHaveCapturingListeners : 1; > uint32_t mMayHaveSystemGroupListeners : 1; > uint32_t mMayHaveAudioAvailableEventListener : 1; > uint32_t mMayHaveTouchEventListener : 1; > uint32_t mMayHaveMouseEnterLeaveEventListener : 1; >+ uint32_t mMayHavePointerEnterLeaveEventListener : 1; > uint32_t mClearingListeners : 1; > uint32_t mNoListenerForEvent : 24; >+ uint32_t mMayHavePointerEventListener : 1; so, you should change 24 to 22. I think that would still be enough for highest event number in nsGUIEvent.h So for various cases in nsEventStateManager we'd need to think about what happen when both pointer and mouse events are dispatched. (since we need to keep supporting mouse events, and we should just add pointer events on top of them.) >+class PointerEnterLeaveDispatcher >+{ >+public: >+ PointerEnterLeaveDispatcher(nsEventStateManager* aESM, >+ nsIContent* aTarget, nsIContent* aRelatedTarget, >+ nsGUIEvent* aEvent, uint32_t aType) >+ : mESM(aESM), mEvent(aEvent), mType(aType) >+ { >+ nsPIDOMWindow* win = >+ aTarget ? aTarget->OwnerDoc()->GetInnerWindow() : nullptr; >+ if (win && win->HasPointerEnterLeaveEventListeners()) { >+ mRelatedTarget = aRelatedTarget ? >+ aRelatedTarget->FindFirstNonChromeOnlyAccessContent() : nullptr; >+ nsINode* commonParent = nullptr; >+ if (aTarget && aRelatedTarget) { >+ commonParent = >+ nsContentUtils::GetCommonAncestor(aTarget, aRelatedTarget); >+ } >+ nsIContent* current = aTarget; >+ // Note, it is ok if commonParent is null! >+ while (current && current != commonParent) { >+ if (!current->ChromeOnlyAccess()) { >+ mTargets.AppendObject(current); >+ } >+ // Pointerenter/leave is fired only on elements. >+ current = current->GetParent(); >+ } >+ } >+ } >+ >+ ~PointerEnterLeaveDispatcher() >+ { >+ if (mType == NS_POINTER_ENTER) { >+ for (int32_t i = mTargets.Count() - 1; i >= 0; --i) { >+ mESM->DispatchPointerEvent(mEvent, mType, mTargets[i], mRelatedTarget); >+ } >+ } else { >+ for (int32_t i = 0; i < mTargets.Count(); ++i) { >+ mESM->DispatchPointerEvent(mEvent, mType, mTargets[i], mRelatedTarget); >+ } >+ } >+ } >+ >+ nsEventStateManager* mESM; >+ nsCOMArray<nsIContent> mTargets; >+ nsCOMPtr<nsIContent> mRelatedTarget; >+ nsGUIEvent* mEvent; >+ uint32_t mType; >+}; Would be good to have just one class for enter/leave handling and use the same for mouse and pointer events >+nsEventStateManager::NotifyPointerOver(nsGUIEvent* aEvent, nsIContent* aContent) >+{ >+ NS_ASSERTION(aContent, "Pointer must be over something"); >+ >+ if (mLastPointerOverElement == aContent) >+ return; >+ >+ // Before firing Pointerover, check for recursion >+ if (aContent == mFirstPointerOverEventElement) >+ return; You're copying code, but still, if (expr) { stmt; } >+nsEventStateManager::GeneratePointerEnterExit(nsGUIEvent* aEvent) Could we try to use the same code for mouse and pointer events. Having the same or almost the same will lead to bugs later when one is changed, but not the other. >+function testPreventDefault() { >+ let event = document.createEvent("PointerEvent"); We don't want this to work. new PointerEvent("pointerdown", {bubbles: true...}); > NS_IMETHODIMP >+Navigator::GetPointerEnabled(bool* aPointerEnabled) >+{ >+ *aPointerEnabled = true; >+ NS_WARNING("Need better pointer events detection"); Well, shouldn't we support mouse pretty much always, and touch often. It is not actually clear why the spec has pointerEnabled. >+ } >+ else if (aEvent->eventStructType == NS_TOUCH_EVENT) } else if (...) { >+ if (pointerMessage) >+ { if (expr) { So the main issues are how mouse and pointer events interact and how they are handled in ESM. In general the code looks good, as I expected :)

> >+ &nsGkAtoms::touch_action, // touch action > Why we need this? that was experiments for touch action css property described in spec... will move it into another bug 795567 > >+ SetIsDOMBinding(); > All events use now Webidl bindings so no need for this. Hmm, why it is still here then? http://mxr.mozilla.org/mozilla-central/source/content/events/src/nsDOMDragEvent.cpp#28 > > > NS_IMETHODIMP > >+Navigator::GetPointerEnabled(bool* aPointerEnabled) > >+{ > >+ *aPointerEnabled = true; > >+ NS_WARNING("Need better pointer events detection"); > Well, shouldn't we support mouse pretty much always, and touch often. > It is not actually clear why the spec has pointerEnabled. IIUC this is just support check, which would allow to not disable runtime js code if browser does not support pointer events

And thanks for detailed review, will fix as much as I can soon

(In reply to Oleg Romashin (MS) from comment #16) > Hmm, why it is still here then? > http://mxr.mozilla.org/mozilla-central/source/content/events/src/ > nsDOMDragEvent.cpp#28 Because the patch is actually still only in m-i and the merge to m-c hasn't happened yet.

Comment on attachment 763013 [details] [diff] [review] WIP Implementation for Pointer events Review of attachment 763013 [details] [diff] [review]: ----------------------------------------------------------------- I'd like to hear gerv's opinion about the changes to license headers here.

Fixed review comments

Hi Oleg and everyone, Microsoft (and any other contributor) retains the copyright to their contributions whether there is an explicit notice to that effect in the file or not. Therefore, while the MPL permits adding additional copyright notices, Mozilla discourages it as it clutters up the file header. Also, it leads to other people wanting to do it. :-) Since we switched to the MPL 2, I personally know of no instance where anyone has done it. If there is some legal or corporate reason why such annotations absolutely must be present, please add them in the following style: /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- * vim: sw=2 ts=2 et : * This Source Code Form is subject to the terms of the Mozilla Public * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. * * Portions Copyright 2013 Microsoft Open Technologies, Inc. */ Please don't say "All Rights Reserved"; it's legal cargo cult text is very misleading, because it appears to contradict the MPL. Gerv

Comment on attachment 763013 [details] [diff] [review] WIP Implementation for Pointer events >diff --git a/content/events/src/nsDOMPointerEvent.cpp b/content/events/src/nsDOMPointerEvent.cpp >new file mode 100644 >--- /dev/null >+++ b/content/events/src/nsDOMPointerEvent.cpp >@@ -0,0 +1,217 @@ >+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ >+/* Copyright (c) Microsoft Open Technologies, Inc. All Rights Reserved. */ >+/* This Source Code Form is subject to the terms of the Mozilla Public >+ * License, v. 2.0. If a copy of the MPL was not distributed with this >+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ >+ >+#include "nsDOMPointerEvent.h" >+ >+nsDOMPointerEvent::nsDOMPointerEvent(mozilla::dom::EventTarget* aOwner, >+ nsPresContext* aPresContext, >+ nsPointerEvent* aEvent) >+ : nsDOMMouseEvent(aOwner, aPresContext, aEvent ? aEvent : new nsPointerEvent(false, 0, nullptr)) >+{ Why don't you use C++ name space |mozilla::dom| like DOM wheel event? http://mxr.mozilla.org/mozilla-central/source/content/events/src/DOMWheelEvent.cpp >diff --git a/widget/nsGUIEvent.h b/widget/nsGUIEvent.h >--- a/widget/nsGUIEvent.h >+++ b/widget/nsGUIEvent.h >@@ -1014,16 +1029,71 @@ public: > reasonType reason : 4; > contextType context : 4; > exitType exit; > > /// The number of mouse clicks > uint32_t clickCount; > }; > >+class nsPointerEvent : public nsMouseEvent Why don't you use |mozilla::widget| like wheel event? We couldn't use name space, but now we can use it.

> We couldn't use name space, but now we can use it. Good point, it was a bit hard to notice wheel event which is seems only has namespace.

Not sure if this is the correct place to raise the question, but: if in the medium term the idea is for the browser to support both touch events (for compatibility with most touch-specific JS out in the wild today) and pointer events at the same time, how will the simulated mouse events come into play? It seems the two event models are fundamentally incompatible in this respect, with touch events firing all synthetic mouse events after touchend touchstart > [touchmove]+ > touchend > mouseover > mousemove > mousedown > click while pointer events dispatch them "inline" with the pointer ones pointerover > mouseover > pointerdown > mousedown > pointermove > mousemove > pointerup > mouseup > pointerout > mouseout > click Additionally, synthetic mouse events and click can be prevented in touch events model by doing a preventDefault, while pointer events make use of the touch-action CSS to suppress synthetic mouse events and explicitly do not allow click to be prevented.

One further complication is the difference in when the classic 300ms delay (unless suppressed) happens: before all the synthetic mouse events in touch, before the click in pointer touchstart > [touchmove]+ > touchend > [300ms delay] > mouseover > mousemove > mousedown > click vs pointerover > mouseover > pointerdown > mousedown > pointermove > mousemove > pointerup > mouseup > pointerout > mouseout > [300ms delay] > click

Fixed most of review comments for original Pointer events definition https://tbpl.mozilla.org/?tree=Try&rev=33b51446c85b

Comment on attachment 767073 [details] [diff] [review] Pointer Events Definition. part 1 I regret to say that "ns" prefix should not be used if the class is defined in non-global name space.

Here is the version of Pointers enter/leave handling which is shared with mouse events, actually it is sharing such properties mFirstMouseOutEventElement, mFirstMouseOverEventElement mLastMouseOverElement it does not work very well because it fail tests https://tbpl.mozilla.org/php/getParsedLog.php?id=25329363&tree=Try and it seems we should create wrapper class for mFirstMouseOutEventElement, mFirstMouseOverEventElement mLastMouseOverElement and track them for each pointer. Need to check how IE works here, and how touch events implemented for :hover

Fixed some problems with constructors and tests https://tbpl.mozilla.org/?tree=Try&rev=476d45dd41e8

Comment on attachment 776448 [details] [diff] [review] Basic Handling of pointer Events This is basic patch which allow to handle simple events down/up/move.

[23:33] smaug romaxa: also, it would be nice to have some information how IE behaves [23:33] smaug especially how pointer events and mouse events work together there [23:34] smaug and which mouse events actually use PointerEvent interface (which extends MouseEvent)

Comment on attachment 776448 [details] [diff] [review] Basic Handling of pointer Events Might be good to have some helper method for (aEvent->eventStructType != NS_MOUSE_EVENT && aEvent->eventStructType != NS_MOUSE_SCROLL_EVENT && aEvent->eventStructType != NS_WHEEL_EVENT && aEvent->eventStructType != NS_POINTER_EVENT && aEvent->eventStructType != NS_DRAG_EVENT && aEvent->eventStructType != NS_SIMPLE_GESTURE_EVENT) For the FIXME, I guess we need to flag either mouseevent for pointerevent that there will be other event coming which will trigger click.

Comment on attachment 777589 [details] [diff] [review] Pointer Events Definition. part 1 remove DOMCI_DATA, and I think we don't need nsIDOMPointerEvent. And you can remove all the relevant methods too. Webidl is enough. Could you call the class PointerEvent, and use header PointerEvent.h. Then you wouldn't need to add anything to Bindings.conf update uuid of nsIInlineEventHandlers. Would like to see a new patch.

Comment on attachment 776445 [details] [diff] [review] Pointer Event Listeners detection Hmm, it is not quite clear to me why we need the stuff in nsGlobalWindow.cpp. Remove them? For touch they were needed because Windows can dispatch only gesture or touch events, not both. I hope it doesn't have the same limitation with pointer events. Please make sure to initialize various bool member variables.

Comment on attachment 777592 [details] [diff] [review] Pointer Events Basic Tests We will need some more tests where event dispatching goes via DOMWindowUtils/Widget level code. But this is ok for the interface. But, could you not use random. Perhaps use some pre-randomfied array of values? That would make debugging later easier.

Fixed comments

Comment on attachment 786169 [details] [diff] [review] Pointer Events Definition. part 1 ># HG changeset patch ># Parent 05d3797276d3d8c7d10ddc66e95c99352c2e3cc9 ># User Oleg Romashin <oleg.romashin@microsoft.com> > >Bug 822898 - Implement pointer events. Interface p1 > >+NS_IMPL_ADDREF_INHERITED(PointerEvent, nsDOMMouseEvent) >+NS_IMPL_RELEASE_INHERITED(PointerEvent, nsDOMMouseEvent) >+ >+NS_INTERFACE_MAP_BEGIN(PointerEvent) >+NS_INTERFACE_MAP_END_INHERITING(nsDOMMouseEvent) I believe you can drop all these macros now if you remove >+ NS_DECL_ISUPPORTS_INHERITED >+ >+ // Forward to base class >+ NS_FORWARD_TO_NSDOMMOUSEEVENT from the .h >+ >+//static >+already_AddRefed<PointerEvent> >+PointerEvent::Constructor(const mozilla::dom::GlobalObject& aGlobal, >+ const nsAString& aType, >+ const mozilla::dom::PointerEventInit& aParam, >+ mozilla::ErrorResult& aRv) >+{ >+ nsCOMPtr<mozilla::dom::EventTarget> t = do_QueryInterface(aGlobal.Get()); >+ nsRefPtr<PointerEvent> e = new PointerEvent(t, nullptr, nullptr); >+ bool trusted = e->Init(t); >+ e->InitPointerEvent(aType, aParam.mBubbles, aParam.mCancelable, >+ aParam.mView, aParam.mDetail, aParam.mScreenX, >+ aParam.mScreenY, aParam.mClientX, aParam.mClientY, >+ aParam.mCtrlKey, aParam.mAltKey, aParam.mShiftKey, >+ aParam.mMetaKey, aParam.mButton, aParam.mRelatedTarget, >+ aParam.mPointerId, aParam.mWidth, aParam.mHeight, >+ aParam.mPressure, aParam.mTiltX, aParam.mTiltY, >+ aParam.mPointerType, aParam.mIsPrimary, >+ aRv); >+ >+ static_cast<widget::PointerEvent*>(e->mEvent)->buttons = aParam.mButtons; >+ e->SetTrusted(trusted); >+ return e.forget(); >+} >+PointerEvent::GetPointerType(nsAString& aPointerType) >+{ >+ ConvertPointerTypeToString(static_cast<mozilla::widget::PointerEvent*>(mEvent)->inputSource, aPointerType); is this overlong line? >+} >+ >+void >+PointerEvent::InitPointerEvent(const nsAString& aType, >+ bool aCanBubble, >+ bool aCancelable, >+ nsIDOMWindow* aView, >+ int32_t aDetail, >+ int32_t aScreenX, >+ int32_t aScreenY, >+ int32_t aClientX, >+ int32_t aClientY, >+ bool aCtrlKey, >+ bool aAltKey, >+ bool aShiftKey, >+ bool aMetaKey, >+ uint16_t aButton, >+ EventTarget* aRelatedTarget, >+ int32_t aPointerId, >+ int32_t aWidth, >+ int32_t aHeight, >+ float aPressure, >+ int32_t aTiltX, >+ int32_t aTiltY, >+ const nsAString& aPointerType, >+ bool aIsPrimary, >+ mozilla::ErrorResult& aRv) >+{ >+ aRv = nsDOMMouseEvent::InitMouseEvent(aType, >+ aCanBubble, >+ aCancelable, >+ aView, >+ aDetail, >+ aScreenX, >+ aScreenY, >+ aClientX, >+ aClientY, >+ aCtrlKey, >+ aAltKey, >+ aShiftKey, >+ aMetaKey, >+ aButton, >+ aRelatedTarget); >+ You should return here if aRv.Failed()... >+ mozilla::widget::PointerEvent* pointerEvent = static_cast<mozilla::widget::PointerEvent*>(mEvent); >+ pointerEvent->pointerId = aPointerId; >+ pointerEvent->width = aWidth; >+ pointerEvent->height = aHeight; >+ pointerEvent->pressure = aPressure; >+ pointerEvent->tiltX = aTiltX; >+ pointerEvent->tiltY = aTiltY; >+ pointerEvent->inputSource = ConvertStringToPointerType(aPointerType); >+ pointerEvent->isPrimary = aIsPrimary; >+} ...but if you don't need to ever initialize these events (in C++), you could drop the whole Init*Event method, and move the pointerEvent initialization to Constructor. >+NS_NewPointerEvent(nsIDOMEvent** aInstancePtrResult, >+ mozilla::dom::EventTarget* aOwner, >+ nsPresContext* aPresContext, >+ class mozilla::widget::PointerEvent* aEvent); Align params >+++ b/dom/webidl/EventHandler.webidl >@@ -121,16 +121,30 @@ interface GlobalEventHandlers { > [SetterThrows] > attribute EventHandler onsuspend; > [SetterThrows] > attribute EventHandler ontimeupdate; > [SetterThrows] > attribute EventHandler onvolumechange; > [SetterThrows] > attribute EventHandler onwaiting; >+ [SetterThrows] >+ attribute EventHandler onpointerdown; >+ [SetterThrows] >+ attribute EventHandler onpointerup; >+ [SetterThrows] >+ attribute EventHandler onpointermove; >+ [SetterThrows] >+ attribute EventHandler onpointerout; >+ [SetterThrows] >+ attribute EventHandler onpointerover; >+ [SetterThrows] >+ attribute EventHandler onpointerenter; >+ [SetterThrows] >+ attribute EventHandler onpointerleave; I think we need PrefControlled for each pointerevent handler and for the interface too, so that we can opt-in having pointer events activated, for now. >+[Constructor(DOMString type, optional PointerEventInit eventInitDict)] >+interface PointerEvent : MouseEvent >+{ >+ readonly attribute long pointerId; >+ readonly attribute long width; >+ readonly attribute long height; >+ readonly attribute float pressure; >+ readonly attribute long tiltX; >+ readonly attribute long tiltY; >+ readonly attribute DOMString pointerType; >+ readonly attribute boolean isPrimary; >+ >+ [Throws] >+ void initPointerEvent(DOMString typeArg, >+ boolean canBubbleArg, >+ boolean cancelableArg, >+ WindowProxy? viewArg, >+ long detailArg, >+ long screenXArg, >+ long screenYArg, >+ long clientXArg, >+ long clientYArg, >+ boolean ctrlKeyArg, >+ boolean altKeyArg, >+ boolean shiftKeyArg, >+ boolean metaKeyArg, >+ unsigned short buttonArg, >+ EventTarget? relatedTargetArg, >+ long pointerIdArg, >+ long widthArg, >+ long heightArg, >+ float presureArg, >+ long tiltXArg, >+ long tiltYArg, >+ DOMString pointerTypeArg, >+ boolean isPrimaryArg); certainly remove initPointerEvent from the .idl. It is not there per spec. Looking good. I wonder if I need to review this again...probably not.

Last comments addressed, added interface enabler by pref dom.w3c_pointer_events.enabled

Ok, one more iteration here, rebased to 0b4b1330fafa m-c Basic interface definition, preference based

This patch is minimal part which allow to dispatch pointer events (up/down/move) via content EventDispatcher pipeline and convert WidgetPointerEvents properties into dom::PointerEvent structure.

It would be nice to get last two patches pushed in sooner, in order to avoid painful rebase-ing in future, also these two patches allow to create and dispatch basic pointer events and open doors for other spec parts implementation without huge patch queue in place

Small update to make test_all_synthetic_events.html test pass.

Comment on attachment 831644 [details] [diff] [review] Pointer Events Basic Tests Oh, sounds like we should define special Powers pref which enable PointerEvents while running dom/tests/mochitest/general/test_interfaces.html

Use SpecialPowers in tests

Try build for last 3 patches: https://tbpl.mozilla.org/?tree=Try&rev=cf89bbec7910

Comment on attachment 831886 [details] [diff] [review] Pointer Events Basic Tests could you add tests also for onfoo properties you're adding to EventHandler.webidl

Comment on attachment 831534 [details] [diff] [review] b822898 p2 pointer events allow dispatch No need for NS_ENSURE_TRUE(pointerEvent, NS_ERROR_OUT_OF_MEMORY); new is infallible.

Added target.onfoo tests

Comment on attachment 8333990 [details] [diff] [review] Pointer Events Basic Tests (We'll get more tests with touch-action.)

remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/a5e360e6d713 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/145150ef00a5 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/63c1f6716755 Don't close this bug when it landed to m-c

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/b159af3edf46 for mochitest-1 bustage on Windows: https://tbpl.mozilla.org/php/getParsedLog.php?id=30725617&tree=Mozilla-Inbound&full=1 15:07:45 INFO - 84701 INFO TEST-INFO | MEMORY STAT largestContiguousVMBlock after test: 589824000 15:07:45 INFO - 84702 INFO TEST-END | /tests/content/events/test/test_bug812744.html | finished in 67ms 15:07:45 INFO - 84703 INFO TEST-START | /tests/content/events/test/test_bug822898.html 15:07:45 INFO - 84704 ERROR TEST-UNEXPECTED-FAIL | /tests/content/events/test/test_bug822898.html | pointerdown property isn't performed 15:07:45 INFO - 84705 ERROR TEST-UNEXPECTED-FAIL | /tests/content/events/test/test_bug822898.html | pointerup property isn't performed 15:07:45 INFO - 84706 ERROR TEST-UNEXPECTED-FAIL | /tests/content/events/test/test_bug822898.html | pointermove property isn't performed 15:07:45 INFO - 84707 ERROR TEST-UNEXPECTED-FAIL | /tests/content/events/test/test_bug822898.html | pointerout property isn't performed 15:07:45 INFO - 84708 ERROR TEST-UNEXPECTED-FAIL | /tests/content/events/test/test_bug822898.html | pointerover property isn't performed 15:07:45 INFO - 84709 ERROR TEST-UNEXPECTED-FAIL | /tests/content/events/test/test_bug822898.html | pointerenter property isn't performed 15:07:45 INFO - 84710 ERROR TEST-UNEXPECTED-FAIL | /tests/content/events/test/test_bug822898.html | pointerleave property isn't performed 15:07:45 INFO - 84711 INFO TEST-PASS | /tests/content/events/test/test_bug822898.html | Correct detail 15:07:45 INFO - 84712 INFO TEST-PASS | /tests/content/events/test/test_bug822898.html | Correct screenX 15:07:45 INFO - 84713 INFO TEST-PASS | /tests/content/events/test/test_bug822898.html | Correct screenY

hmm this looks strange, looks like some problem with special powers in this case applying on .onfoo properties... it works on Linux build, it works if I set "dom.w3c_pointer_events.enabled" -> true in all.js on windows https://tbpl.mozilla.org/?tree=Try&rev=083eb9504e0f Cannot test windows build because it does not compile on my environment, so if anyone has any suggestions, please advice

The pref change will not be reflected to existing global objects. You will have to create a new global object (e.g. by inserting an iframe) to test the default-disabled interfaces or properties.

This looks much better https://tbpl.mozilla.org/?tree=Try&rev=88042f6343bd

Workaround for fixing this build failure on windows https://tbpl.mozilla.org/php/getParsedLog.php?id=30843057&tree=Try#error0

now it looks ok: https://tbpl.mozilla.org/?tree=Try&rev=727a43aa1cd1

remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/971c64fc4e17 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/36431dbba2de remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/25c21fd41691 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/384337ee73f4

https://hg.mozilla.org/mozilla-central/rev/596ed05b32f6 https://hg.mozilla.org/mozilla-central/rev/5721f317bab1

ctor tests moved into same file, so we have one tests file covered with pointer enabler preference, and fixing this problem https://tbpl.mozilla.org/php/getParsedLog.php?id=30879131&tree=Try#error0 Load style and preference enabler changed in order to fix issues on B2G emulator https://tbpl.mozilla.org/php/getParsedLog.php?id=30855965&tree=Mozilla-Inbound#error0

Comment on attachment 8336521 [details] [diff] [review] Updated Pointer Events Basic Tests. Try build with all tests, all platforms enabled: https://tbpl.mozilla.org/?tree=Try&rev=1926b22e35e7

Added attributes to the navigator interface according to the http://www.w3.org/TR/pointerevents/#extensions-to-the-navigator-interface. Intentionally omitted pointerEnabled attribute since it was removed recently (reference: https://www.w3.org/Bugs/Public/show_bug.cgi?id=22890).

Comment on attachment 8362917 [details] [diff] [review] Add maxTouchPoints attribute to the navigator interface >+Navigator::MaxTouchPoints() >+{ >+ nsCOMPtr<nsIWidget> widget = widget::WidgetUtils::DOMWindowToWidget(mWindow); DOMWindowToWidget may return nullptr, I believe we don't want to have possibility crash here >+ return widget->GetMaxTouchPoints(); >+} >+ > >+uint32_t nsBaseWidget::GetMaxTouchPoints() const >+{ >+ return 0; Do we have any specification on what should be default value here when MaxTouchPoints info retreival not supported by platform? should it be -1 - indicating as not implemented or 0? >- nsIWidget *self = static_cast<nsIWidget *>(aClosure); >+ nsIWidget *self = static_cast<nsIWidget *>(aClosure); do we really need this line change?

(In reply to Oleg Romashin (:romaxa) from comment #68) > Comment on attachment 8362917 [details] [diff] [review] > Add maxTouchPoints attribute to the navigator interface > > >+Navigator::MaxTouchPoints() > >+{ > >+ nsCOMPtr<nsIWidget> widget = widget::WidgetUtils::DOMWindowToWidget(mWindow); > > DOMWindowToWidget may return nullptr, I believe we don't want to have > possibility crash here > > >+ return widget->GetMaxTouchPoints(); > >+} > >+ > Ok. In most places where DOMWindowToWidget is used we use assert that ensures that widget is not null. E.g. http://mxr.mozilla.org/mozilla-central/source/widget/xpwidgets/nsBaseFilePicker.cpp#134, http://mxr.mozilla.org/mozilla-central/source/dom/plugins/ipc/PluginHangUIParent.cpp#392. Should I use assert too or simple check for null is fine? > > > >+uint32_t nsBaseWidget::GetMaxTouchPoints() const > >+{ > >+ return 0; > > Do we have any specification on what should be default value here when > MaxTouchPoints info retreival not supported by platform? > should it be -1 - indicating as not implemented or 0? There is no info about it in the spec so i just left 0 here. Is it Ok? > >- nsIWidget *self = static_cast<nsIWidget *>(aClosure); > >+ nsIWidget *self = static_cast<nsIWidget *>(aClosure); > > do we really need this line change? Nope, will fix this nit.

> Ok. In most places where DOMWindowToWidget is used we use assert that > ensures that widget is not null. E.g. in addition we may want to do same as Navigator::AddIdleObserver if (!widget) { aRv.Throw(NS_ERROR_UNEXPECTED); return; }

Addressed core review comments.

Comment on attachment 8365025 [details] [diff] [review] Add maxTouchPoints attribute to the navigator interface. v2. Update IID of nsIWidget. maxTouchPoints is not supposed to Throw. Could we perhaps just return 0 in case we can't access widget. How slow or fast MetroWidget::GetMaxTouchPoints() is ?

Addressed review comments. Also carrying r+.

MaxTouchPoints API https://hg.mozilla.org/integration/mozilla-inbound/rev/fe77d4ae3d50

With pointer events still a work-in-progress (and as a newbie to Mozilla development), I hope its not bad form to submit a bugfix for one of the comitted patches here rather than opening a new bug... While hacking on a GTK implementation for #956644, I noticed issues reporting pen pressure. It looks like the return type of PointerEvent::Pressure was accidentally set to 'int32_t' instead of the expected 'float'.

A test for that would be good.

For reference, we're tagging front end bugs in the metro browser related to pointer events with [pointer-events]. https://bugzilla.mozilla.org/buglist.cgi?list_id=9403886&resolution=---&resolution=DUPLICATE&status_whiteboard_type=allwordssubstr&query_format=advanced&status_whiteboard=[pointer-events]

bug 1015600 suggests we're shipping a somewhat half-baked pointer events implementation, which causes site problems..?

Recent (and sad) news is that Chrome team decided not to support Pointer Events: https://code.google.com/p/chromium/issues/detail?id=162757#c64

https://bugzilla.mozilla.org/show_bug.cgi?id=1071554 and https://bugzilla.mozilla.org/show_bug.cgi?id=1071553 are about the CSS pointer-events property...which has nothing to do with the Pointer Events spec. Please remove the dependency.

Thanks, Patrick. I thought I was going crazy here...

no worries, the naming clash is confusing to many :)

Chrome just decided to implement this: https://code.google.com/p/chromium/issues/detail?id=162757#c146

Hello, as I couldn't find something to test pen pressure, I did this https://jsfiddle.net/kgkf5kfx/ When using a digitizer pen on Windows, it should display a number between 0 and 1000 when scribbling on the result window. It works correctly with IE11 but in FF (with flag enabled) it's always 0 or 500.

(In reply to kungfoobar from comment #89) > When using a digitizer pen on Windows, it should display a number between 0 > and 1000 when scribbling on the result window. It works correctly with IE11 > but in FF (with flag enabled) it's always 0 or 500. There is bug 1031362 about it.