Last Comment Bug 822898 - Implement pointer events
: Implement pointer events
Status: NEW
[leave-open]
: dev-doc-needed
Product: Core
Classification: Components
Component: DOM: Events (show other bugs)
: unspecified
: All All
: -- normal with 25 votes (vote)
: ---
Assigned To: Oleg Romashin (MS)
:
Mentors:
http://dvcs.w3.org/hg/pointerevents/r...
Depends on: 966961 1122211 1159206 795567 850630 956644 967796 968148 970964 974305 977695 979497 985511 987078 996493 1066237 1137555 1149861 1153130 1153135 1162009
Blocks: html5test 960316 1171711
  Show dependency treegraph
 
Reported: 2012-12-18 16:44 PST by Olli Pettay [:smaug] (high review load, please consider other reviewers)
Modified: 2016-05-16 01:02 PDT (History)
63 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
WIP Implementation for Pointer events (160.27 KB, patch)
2013-06-14 17:22 PDT, Oleg Romashin (MS)
bugs: feedback-
Details | Diff | Review
Pointer Events Definition. part 1 (35.07 KB, patch)
2013-06-18 00:53 PDT, Oleg Romashin (MS)
no flags Details | Diff | Review
Pointer Events Tests (Quick WIP). part 2 (9.75 KB, patch)
2013-06-18 00:54 PDT, Oleg Romashin (MS)
no flags Details | Diff | Review
Pointer Events. Navigator Props. part 3 (1.48 KB, patch)
2013-06-18 00:54 PDT, Oleg Romashin (MS)
no flags Details | Diff | Review
Pointer Events. Window/Widget notifications. part 4 (17.82 KB, patch)
2013-06-18 00:55 PDT, Oleg Romashin (MS)
no flags Details | Diff | Review
Pointer Events. Set Capture wip. part 5 (4.85 KB, patch)
2013-06-18 00:57 PDT, Oleg Romashin (MS)
no flags Details | Diff | Review
Pointer Events. Content/Dom/Layout/Widget. part 6 (58.85 KB, patch)
2013-06-18 00:58 PDT, Oleg Romashin (MS)
no flags Details | Diff | Review
Pointer Events. Convert Mouse/Touch -> Pointers. part 7 (4.55 KB, patch)
2013-06-18 00:59 PDT, Oleg Romashin (MS)
no flags Details | Diff | Review
Pointer Events Definition. part 1 (35.20 KB, patch)
2013-06-21 21:21 PDT, Oleg Romashin (:romaxa)
no flags Details | Diff | Review
Pointer Events Definition. part 1 (36.58 KB, patch)
2013-06-25 00:37 PDT, Oleg Romashin (MS)
no flags Details | Diff | Review
Pointer Events Definition. part 1 (36.24 KB, patch)
2013-07-16 09:01 PDT, Oleg Romashin (:romaxa)
no flags Details | Diff | Review
Pointer Event Listeners detection (17.48 KB, patch)
2013-07-16 09:02 PDT, Oleg Romashin (:romaxa)
bugs: review-
Details | Diff | Review
Basic Handling of pointer Events (20.70 KB, patch)
2013-07-16 09:04 PDT, Oleg Romashin (:romaxa)
bugs: feedback+
Details | Diff | Review
Enter/Leave version shared with mouse (10.22 KB, patch)
2013-07-16 09:11 PDT, Oleg Romashin (MS)
no flags Details | Diff | Review
Pointer Events Definition. part 1 (36.31 KB, patch)
2013-07-17 22:17 PDT, Oleg Romashin (MS)
bugs: review-
Details | Diff | Review
Pointer Events Basic Tests (10.46 KB, patch)
2013-07-17 22:19 PDT, Oleg Romashin (MS)
bugs: review+
Details | Diff | Review
Pointer Events Definition. part 1 (31.58 KB, patch)
2013-08-06 00:20 PDT, Oleg Romashin (MS)
bugs: review+
Details | Diff | Review
Pointer Events Definition. part 1 (27.31 KB, patch)
2013-08-06 09:05 PDT, Oleg Romashin (:romaxa)
no flags Details | Diff | Review
Pointer Events Definition p1. basic pointer event processing (25.88 KB, patch)
2013-11-13 07:27 PST, Oleg Romashin (MS)
bugs: review+
Details | Diff | Review
b822898 p2 pointer events allow dispatch (12.63 KB, patch)
2013-11-13 07:31 PST, Oleg Romashin (MS)
bugs: review+
Details | Diff | Review
Pointer Events Basic Tests (12.67 KB, patch)
2013-11-13 10:05 PST, Oleg Romashin (MS)
no flags Details | Diff | Review
Pointer Events Basic Tests (13.17 KB, patch)
2013-11-13 16:01 PST, Oleg Romashin (MS)
bugs: review-
Details | Diff | Review
Pointer Events Basic Tests (14.53 KB, patch)
2013-11-18 11:10 PST, Oleg Romashin (MS)
bugs: review+
Details | Diff | Review
Updated Pointer Events Basic Tests. (14.79 KB, patch)
2013-11-19 21:12 PST, Oleg Romashin (MS)
no flags Details | Diff | Review
Create Event problem workaround (784 bytes, patch)
2013-11-20 11:04 PST, Oleg Romashin (MS)
ehsan: review+
Details | Diff | Review
Updated Pointer Events Basic Tests. (14.59 KB, patch)
2013-11-21 18:50 PST, Oleg Romashin (MS)
bugs: review+
Details | Diff | Review
Add maxTouchPoints attribute to the navigator interface (11.27 KB, patch)
2014-01-21 04:15 PST, Nick Lebedev [:nl]
romaxa: review-
Details | Diff | Review
Add maxTouchPoints attribute to the navigator interface. v2. (10.75 KB, patch)
2014-01-24 04:59 PST, Nick Lebedev [:nl]
romaxa: review+
bugs: review+
Details | Diff | Review
Add maxTouchPoints attribute to navigator. (11.29 KB, patch)
2014-01-28 05:49 PST, Nick Lebedev [:nl]
nicklebedev37: review+
Details | Diff | Review
PointerEvent::Pressure should return float (1.26 KB, patch)
2014-02-06 10:54 PST, killertofu
bugs: review+
Details | Diff | Review

Description Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-12-18 16:44:49 PST
The API is still very my in flux, but we should start implementing it soon.
Comment 1 Tantek Çelik 2013-03-15 17:38:59 PDT
cc: Matt Brubeck for input.
Comment 2 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2013-03-15 18:02:26 PDT
For what input ? :)

We should implement pointer events, and the spec is getting more stable.
Comment 3 Matt Brubeck (:mbrubeck) 2013-03-18 09:17:53 PDT
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).
Comment 4 Oleg Romashin (MS) 2013-06-14 17:22:16 PDT
Created attachment 763013 [details] [diff] [review]
WIP Implementation for Pointer events

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.
Comment 5 Oleg Romashin (MS) 2013-06-18 00:53:09 PDT
Created attachment 764019 [details] [diff] [review]
Pointer Events Definition. part 1
Comment 6 Oleg Romashin (MS) 2013-06-18 00:54:12 PDT
Created attachment 764020 [details] [diff] [review]
Pointer Events Tests (Quick WIP). part 2
Comment 7 Oleg Romashin (MS) 2013-06-18 00:54:57 PDT
Created attachment 764021 [details] [diff] [review]
Pointer Events. Navigator Props. part 3
Comment 8 Oleg Romashin (MS) 2013-06-18 00:55:52 PDT
Created attachment 764022 [details] [diff] [review]
Pointer Events. Window/Widget notifications. part 4
Comment 9 Oleg Romashin (MS) 2013-06-18 00:57:09 PDT
Created attachment 764023 [details] [diff] [review]
Pointer Events. Set Capture wip. part 5
Comment 10 Oleg Romashin (MS) 2013-06-18 00:58:28 PDT
Created attachment 764024 [details] [diff] [review]
Pointer Events. Content/Dom/Layout/Widget. part 6
Comment 11 Oleg Romashin (MS) 2013-06-18 00:59:42 PDT
Created attachment 764025 [details] [diff] [review]
Pointer Events. Convert Mouse/Touch -> Pointers. part 7
Comment 12 ofirr.dev 2013-06-18 15:15:52 PDT
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.
Comment 13 Oleg Romashin (MS) 2013-06-18 15:36:16 PDT
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...
Comment 14 ofirr.dev 2013-06-18 16:03:21 PDT
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 15 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2013-06-20 14:29:03 PDT
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 :)
Comment 16 Oleg Romashin (MS) 2013-06-20 14:54:25 PDT
> >+  &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
Comment 17 Oleg Romashin (MS) 2013-06-20 14:55:10 PDT
And thanks for detailed review, will fix as much as I can soon
Comment 18 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2013-06-20 14:59:18 PDT
(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 19 :Ms2ger 2013-06-21 00:13:31 PDT
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.
Comment 20 Oleg Romashin (:romaxa) 2013-06-21 21:21:14 PDT
Created attachment 766226 [details] [diff] [review]
Pointer Events Definition. part 1

Fixed review comments
Comment 21 Gervase Markham [:gerv] 2013-06-24 02:48:46 PDT
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 22 Masayuki Nakano [:masayuki] (Mozilla Japan) 2013-06-24 05:05:57 PDT
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.
Comment 23 Oleg Romashin (MS) 2013-06-24 05:11:16 PDT
> 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.
Comment 24 Patrick H. Lauke 2013-06-24 12:05:48 PDT
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.
Comment 25 Patrick H. Lauke 2013-06-24 16:59:45 PDT
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
Comment 26 Oleg Romashin (MS) 2013-06-25 00:37:45 PDT
Created attachment 767073 [details] [diff] [review]
Pointer Events Definition. part 1

Fixed most of review comments for original Pointer events definition
https://tbpl.mozilla.org/?tree=Try&rev=33b51446c85b
Comment 27 Masayuki Nakano [:masayuki] (Mozilla Japan) 2013-06-25 02:00:22 PDT
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.
Comment 28 Oleg Romashin (:romaxa) 2013-07-16 09:01:19 PDT
Created attachment 776444 [details] [diff] [review]
Pointer Events Definition. part 1
Comment 29 Oleg Romashin (:romaxa) 2013-07-16 09:02:45 PDT
Created attachment 776445 [details] [diff] [review]
Pointer Event Listeners detection
Comment 30 Oleg Romashin (:romaxa) 2013-07-16 09:04:59 PDT
Created attachment 776448 [details] [diff] [review]
Basic Handling of pointer Events
Comment 31 Oleg Romashin (MS) 2013-07-16 09:11:18 PDT
Created attachment 776451 [details] [diff] [review]
Enter/Leave version shared with mouse

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
Comment 32 Oleg Romashin (MS) 2013-07-17 22:17:15 PDT
Created attachment 777589 [details] [diff] [review]
Pointer Events Definition. part 1

Fixed some problems with constructors and tests
https://tbpl.mozilla.org/?tree=Try&rev=476d45dd41e8
Comment 33 Oleg Romashin (MS) 2013-07-17 22:19:08 PDT
Created attachment 777592 [details] [diff] [review]
Pointer Events Basic Tests
Comment 34 Oleg Romashin (:romaxa) 2013-07-17 22:24:08 PDT
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.
Comment 35 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2013-07-19 13:37:37 PDT
[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 36 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2013-07-22 06:59:13 PDT
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 37 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2013-08-05 12:17:10 PDT
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 38 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2013-08-05 13:00:39 PDT
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 39 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2013-08-05 13:08:24 PDT
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.
Comment 40 Oleg Romashin (MS) 2013-08-06 00:20:56 PDT
Created attachment 786169 [details] [diff] [review]
Pointer Events Definition. part 1

Fixed comments
Comment 41 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2013-08-06 04:03:55 PDT
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.
Comment 42 Oleg Romashin (:romaxa) 2013-08-06 09:05:11 PDT
Created attachment 786330 [details] [diff] [review]
Pointer Events Definition. part 1

Last comments addressed, added interface enabler by pref
dom.w3c_pointer_events.enabled
Comment 43 Oleg Romashin (MS) 2013-11-13 07:27:40 PST
Created attachment 831533 [details] [diff] [review]
Pointer Events Definition p1. basic pointer event processing

Ok, one more iteration here, rebased to 0b4b1330fafa m-c
Basic interface definition, preference based
Comment 44 Oleg Romashin (MS) 2013-11-13 07:31:05 PST
Created attachment 831534 [details] [diff] [review]
b822898 p2 pointer events allow dispatch

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.
Comment 45 Oleg Romashin (MS) 2013-11-13 08:22:45 PST
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
Comment 46 Oleg Romashin (MS) 2013-11-13 10:05:57 PST
Created attachment 831644 [details] [diff] [review]
Pointer Events Basic Tests

Small update to make test_all_synthetic_events.html test pass.
Comment 47 Oleg Romashin (MS) 2013-11-13 10:55:17 PST
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
Comment 48 Oleg Romashin (MS) 2013-11-13 16:01:18 PST
Created attachment 831886 [details] [diff] [review]
Pointer Events Basic Tests

Use SpecialPowers in tests
Comment 49 Oleg Romashin (MS) 2013-11-13 16:03:42 PST
Try build for last 3 patches:
https://tbpl.mozilla.org/?tree=Try&rev=cf89bbec7910
Comment 50 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2013-11-15 15:28:46 PST
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 51 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2013-11-15 15:44:18 PST
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.
Comment 52 Oleg Romashin (MS) 2013-11-18 11:10:35 PST
Created attachment 8333990 [details] [diff] [review]
Pointer Events Basic Tests

Added target.onfoo tests
Comment 53 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2013-11-18 11:17:08 PST
Comment on attachment 8333990 [details] [diff] [review]
Pointer Events Basic Tests

(We'll get more tests with touch-action.)
Comment 55 Wes Kocher (:KWierso) 2013-11-18 15:30:07 PST
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
Comment 56 Oleg Romashin (MS) 2013-11-19 09:32:17 PST
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
Comment 57 Masatoshi Kimura [:emk] 2013-11-19 20:04:25 PST
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.
Comment 58 Oleg Romashin (MS) 2013-11-19 21:12:34 PST
Created attachment 8335097 [details] [diff] [review]
Updated Pointer Events Basic Tests.

This looks much better
https://tbpl.mozilla.org/?tree=Try&rev=88042f6343bd
Comment 59 Oleg Romashin (MS) 2013-11-20 11:04:49 PST
Created attachment 8335452 [details] [diff] [review]
Create Event problem workaround

Workaround for fixing this build failure on windows
https://tbpl.mozilla.org/php/getParsedLog.php?id=30843057&tree=Try#error0
Comment 60 Oleg Romashin (MS) 2013-11-20 13:00:48 PST
now it looks ok:
https://tbpl.mozilla.org/?tree=Try&rev=727a43aa1cd1
Comment 63 Oleg Romashin (MS) 2013-11-21 18:50:23 PST
Created attachment 8336521 [details] [diff] [review]
Updated Pointer Events Basic Tests.

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 64 Oleg Romashin (MS) 2013-11-21 18:53:39 PST
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
Comment 66 Ryan VanderMeulen [:RyanVM] 2013-11-22 12:26:59 PST
https://hg.mozilla.org/mozilla-central/rev/c26abb79ec82
Comment 67 Nick Lebedev [:nl] 2014-01-21 04:15:56 PST
Created attachment 8362917 [details] [diff] [review]
Add maxTouchPoints attribute to the navigator interface

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 68 Oleg Romashin (:romaxa) 2014-01-21 04:32:40 PST
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?
Comment 69 Nick Lebedev [:nl] 2014-01-21 04:45:51 PST
(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.
Comment 70 Oleg Romashin (:romaxa) 2014-01-23 07:55:40 PST
> 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;
  }
Comment 71 Nick Lebedev [:nl] 2014-01-24 04:59:02 PST
Created attachment 8365025 [details] [diff] [review]
Add maxTouchPoints attribute to the navigator interface. v2.

Addressed core review comments.
Comment 72 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2014-01-24 13:14:14 PST
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 ?
Comment 73 Nick Lebedev [:nl] 2014-01-28 05:49:19 PST
Created attachment 8366612 [details] [diff] [review]
Add maxTouchPoints attribute to navigator.

Addressed review comments. Also carrying r+.
Comment 74 Oleg Romashin (MS) 2014-01-29 07:24:20 PST
MaxTouchPoints API
https://hg.mozilla.org/integration/mozilla-inbound/rev/fe77d4ae3d50
Comment 75 Ryan VanderMeulen [:RyanVM] 2014-01-29 12:16:02 PST
https://hg.mozilla.org/mozilla-central/rev/fe77d4ae3d50
Comment 76 killertofu 2014-02-06 10:54:40 PST
Created attachment 8371670 [details] [diff] [review]
PointerEvent::Pressure should return float

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'.
Comment 77 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2014-02-06 11:29:09 PST
A test for that would be good.
Comment 78 Ryan VanderMeulen [:RyanVM] 2014-02-07 11:26:50 PST
https://hg.mozilla.org/mozilla-central/rev/5d69f43260ef
Comment 79 Jim Mathies [:jimm] 2014-02-10 05:19:26 PST
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]
Comment 80 Ryan VanderMeulen [:RyanVM] 2014-02-11 11:53:16 PST
https://hg.mozilla.org/mozilla-central/rev/4caebc3011d2
Comment 83 Hallvord R. M. Steen [:hallvors] 2014-05-28 06:52:45 PDT
bug 1015600 suggests we're shipping a somewhat half-baked pointer events implementation, which causes site problems..?
Comment 84 merihakar 2014-08-15 09:14:02 PDT
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
Comment 85 Patrick H. Lauke 2014-09-24 01:36:10 PDT
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.
Comment 86 Timwi 2014-09-24 02:40:51 PDT
Thanks, Patrick. I thought I was going crazy here...
Comment 87 Patrick H. Lauke 2014-09-24 02:42:15 PDT
no worries, the naming clash is confusing to many :)
Comment 88 warcraftthreeft 2015-03-25 09:35:02 PDT
Chrome just decided to implement this: https://code.google.com/p/chromium/issues/detail?id=162757#c146
Comment 89 kungfoobar 2015-04-04 15:57:57 PDT
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.
Comment 90 Maksim Lebedev 2015-04-06 01:31:12 PDT
(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.

Note You need to log in before you can comment on or make changes to this bug.