Expose mouse event pressure

RESOLVED FIXED

Status

()

Core
DOM: Events
RESOLVED FIXED
10 years ago
4 years ago

People

(Reporter: dougt, Assigned: romaxa)

Tracking

(Depends on: 1 bug, {dev-doc-complete, mobile})

unspecified
x86
Linux
dev-doc-complete, mobile
Points:
---
Dependency tree / graph
Bug Flags:
wanted1.9.1 ?
wanted-fennec1.0 +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(4 attachments, 9 obsolete attachments)

20.80 KB, patch
Details | Diff | Splinter Review
2.12 KB, text/plain
Details
19.30 KB, patch
smaug
: review+
Details | Diff | Splinter Review
21.75 KB, patch
Details | Diff | Splinter Review
(Reporter)

Description

10 years ago
On some devices (including tablets), the amount of pressure used when tapping on the screen is available to application programs.  This information can be used for a variety of applications.  I proposed that we expose this pressure information into the DOM through the MouseEvent in some way.

I have put together a work-in-progress patch which exposes this value on GTK.  And have a bunch of questions:

1) jst, is this the right way to extend into the DOM this pressure value

2) How should we determine which devices do and do not have pressure information available?  In the patch, i am using a #ifdef and that seems wrong.

3) gtk2 hackers, is there a better way to enable additional data instead of marking each new window?

4) what should the pressure value be?  I was thinking it would be a float ratio between 0 and 100.  In the patch, i am wrongly casting from GTK's double to a float.  

5) what do we have to do to proposed this addition to the DOM.  Webkit has some discussion here: https://bugs.webkit.org/show_bug.cgi?id=10598.  I think that might an overkill for most devices, but am not sure.
(Reporter)

Comment 1

10 years ago
Created attachment 315584 [details] [diff] [review]
w-i-p patch
(Reporter)

Comment 2

10 years ago
Created attachment 315586 [details]
w-i-p new interface: mozilla/dom/public/idl/events/nsIDOMMouseWithPressureEvent.idl
(Reporter)

Updated

10 years ago

Comment 3

10 years ago
You could bring up the idea to W3C Web API WG for DOM 3 Events, though
that won't be a very fast process.

If you're going to extend current mouse events with a new property, might
be better to use moz -prefix and clearly document that the API isn't frozen.

Comment 4

10 years ago
And if W3C specifies something, it should probably be something that could be 
used to create InkML http://www.w3.org/TR/InkML/.
(Reporter)

Updated

10 years ago
Keywords: mobile

Comment 5

9 years ago
dougt,

(In reply to comment #0)
> On some devices (including tablets), the amount of pressure used when tapping
> on the screen is available to application programs.  This information can be
> used for a variety of applications.  I proposed that we expose this pressure
> information into the DOM through the MouseEvent in some way.

I'll let jst address how *we* should be exposing stuff in DOM, but can you elaborate on "this information can be used for a variety of applications."  I'm willing to add this to the roster of discussion topics at WebAPI, modulo good use cases :)

> 2) How should we determine which devices do and do not have pressure
> information available?  In the patch, i am using a #ifdef and that seems wrong.

The devil is in these kinds of details.  Perhaps a strawman in WebIDL (analogous to say how objects like http://www.w3.org/TR/XMLHttpRequest/#xmlhttprequest are defined) might be helpful?  WebKit guys do something similar in https://bugs.webkit.org/show_bug.cgi?id=10598 which you make reference to, e.g. testing for the existence of a (possible float) readonly .pressure property?  Something like that?  

> 4) what should the pressure value be?  I was thinking it would be a float ratio
> between 0 and 100.  In the patch, i am wrongly casting from GTK's double to a
> float.  
> 

Typically, it's good to take *existing practices* and reuse them.  Can you cite any existing examples which do have a pressure value?  If none exist, the folks that come to WebAPI WG do include well-known UAs and well-known devices, so it's worth asking.

> 5) what do we have to do to proposed this addition to the DOM.  Webkit has some
> discussion here: https://bugs.webkit.org/show_bug.cgi?id=10598.  I think that
> might an overkill for most devices, but am not sure.
> 

Those guys also want to add to JSMouseEvent, similar to what you want to do.  I think that these discussion ought to be connected, since we shouldn't diverge dramatically from their thinking on the subject.  Thus, I'm not sure it's overkill per se.  Even if it was overkill, if anything like that gets standardized (or is assumed as a defacto standard), we'd want to think about a subset of it as a patch :)
(Assignee)

Comment 6

9 years ago
+ gtk_widget_set_extension_events (mShell, GDK_EXTENSION_EVENTS_ALL);
this is wrong.
If you receiving events from mContainer then set_extension_events need to be called from mContainer.
(Assignee)

Comment 7

9 years ago
With this API + spatial-navigation API we can create cool feature for mobile...

Some time users try to click some links on the page by finger and they are clicking near the link at empty area... as result they have to try click again.

We can read pressure value and ask spatial-navigation to search some elements near target X,Y... and resend event again to the element near X,Y.

It should improve mozilla fingerability...
(Reporter)

Comment 8

9 years ago
romaxa, great idea.  could you fix up the set_extension_events patch?  I can look into the spatial nav stuff in another bug (449165).

Comment 9

9 years ago
Comment on attachment 315586 [details]
w-i-p new interface: mozilla/dom/public/idl/events/nsIDOMMouseWithPressureEvent.idl

Would it make sense to add const float MIN_PRESSURE = some_small_value;
and
const float MAX_PRESSURE = some_larger_value;
and then normalize the pressure to be always between those limits?
(Reporter)

Comment 10

9 years ago
if it isn't totally clear, the patch posted is a work-in-progress and isn't ready for review.  Thanks for the comments; keep um coming!

smaug, what does that buy us?
Actually the max pressure could be enough, so the value would be
anything between ]0, MAX_PRESSURE]
We need to have some limits so that based on the pressure level scripts can
do something reasonable.

What kind of data does GDK give us? What about OSX or Win/WinCE?
(Assignee)

Comment 12

9 years ago
>What kind of data does GDK give us?
N810:
0.0 - light touch with stylus
0.1-0.2 - normal stylus touch
0.2-0.5 - normal finger press
0.5-0.75 - Fat thumb press
0.75 - 0.99 - palm press
Do you mean this data?

Updated

9 years ago
Flags: wanted1.9.1?
(Assignee)

Comment 13

9 years ago
Created attachment 335753 [details] [diff] [review]
Updated nsWindow.cpp part (according to comment #7)
Would it make sense to use the same event type for this and Bug 412486?
(Assignee)

Comment 15

9 years ago
Created attachment 337281 [details] [diff] [review]
New updated version

Updated to latest trunk.
Added motion events filtering. See http://library.gnome.org/devel/gdk/unstable/gdk-Events.html#GDK-POINTER-MOTION-HINT-MASK:CAPS
Added ignoring of non-related events and using their pressure value.

Current way looks like hack:
..event->axes = &pressure;
..event.pressure = aEvent->axes?*aEvent->axes:0;

But I have another version of patch with new argument for functions
OnMotionNotifyEvent, OnButtonPressEvent, OnButtonReleaseEvent

https://garage.maemo.org/svn/browser/mozilla/trunk/microb-engine/microb-engine/debian/patches/BMO_attached/090_bug428988_pressure_v2.diff
Attachment #337281 - Flags: review?(doug.turner)
(Reporter)

Updated

9 years ago
Attachment #337281 - Flags: review?(jst)
(Reporter)

Comment 16

9 years ago
Comment on attachment 337281 [details] [diff] [review]
New updated version

i can't review this since I wrote most of it.
Attachment #337281 - Flags: review?(doug.turner) → review?(roc)
(Assignee)

Comment 17

9 years ago
Doug but what about that hacky using of envent->axes value?

Btw:
button_release/press_event_cb(GtkWidget *widget, GdkEventButton *event) 
+    if (event->axes) { 
+      gdk_event_get_axis ((GdkEvent*)event, GDK_AXIS_PRESSURE, &pressure); 
+      return FALSE; - this is need to be removed because it break events...
(Reporter)

Comment 18

9 years ago
romaxa, yeah, i do not think that this patch is close to being something we can checkin, but would like to also hear from roc and jst about the approach and thoughts on exposing "pressure" on the mouse event.
(Assignee)

Comment 19

9 years ago
Created attachment 337675 [details] [diff] [review]
New updated version + broken gtk events fix comment 17

Here is the updated version.
I don't think you need a separate event type. Why not just add pressure and whatever else to normal mouse events? At most you would need to add an nsIDOMNSMouseEvent interface to carry those attributes. Or you could add them to nsIDOMNSUIEvent.

In principle the overall idea sounds fine to me but if possible I'd like you to do an inventory of various platforms and what they support here. That would include Win32, iPhone and Qt as well as GTK. Then we can get an idea of what should be in the API.

Updated

9 years ago
Attachment #337281 - Flags: review?(jst) → review?(Olli.Pettay)
Comment on attachment 337281 [details] [diff] [review]
New updated version

Smaug, could you review this patch?
So which patch should be reviewed? 
https://bugzilla.mozilla.org/attachment.cgi?id=337281
or
https://bugzilla.mozilla.org/attachment.cgi?id=337675
(Assignee)

Comment 23

9 years ago
This one:
https://bugzilla.mozilla.org/attachment.cgi?id=337675
Well, the main thing here is to address comment #20.
Comment on attachment 337675 [details] [diff] [review]
New updated version + broken gtk events fix comment 17


>+  float  pressure = 0;
>+
>   if (aSourceEvent->eventStructType == NS_MOUSE_EVENT) {
>     clickCount = static_cast<nsMouseEvent*>(aSourceEvent)->clickCount;
>-  }
>+    pressure   = static_cast<nsMouseEvent*>(aSourceEvent)->pressure;
>+  }
nit, 2 extra space after pressure.

> nsresult NS_NewDOMMouseEvent(nsIDOMEvent** aInstancePtrResult,
>                              nsPresContext* aPresContext,
>                              nsInputEvent *aEvent) 
> {
>+#define HAS_MOUSE_PRESSURE
>+#ifdef HAS_MOUSE_PRESSURE
>+  nsDOMMouseWithPressureEvent* it = new nsDOMMouseWithPressureEvent(aPresContext, aEvent);
>+#else
>   nsDOMMouseEvent* it = new nsDOMMouseEvent(aPresContext, aEvent);
>+#endif
>   if (nsnull == it) {
>     return NS_ERROR_OUT_OF_MEMORY;
>   }
So either we should always have .pressure or never. No #ifdefs

>@@ -78,16 +78,17 @@ enum nsDOMClassInfoID {
>   // StyleSheet classes
>   eDOMClassInfo_DocumentStyleSheetList_id,
> 
>   // Event classes
>   eDOMClassInfo_Event_id,
>   eDOMClassInfo_MutationEvent_id,
>   eDOMClassInfo_UIEvent_id,
>   eDOMClassInfo_MouseEvent_id,
>+  eDOMClassInfo_MouseWithPressureEvent_id,
This should be added to the end of the list.
Similar thing in nsDOMClassInfo.cpp

>+    // should we move this into !synthEvent?
>+    event.pressure = aEvent->axes?*aEvent->axes:0;
>+
Space before and after ? and :
Also elsewhere.

> void
> nsWindow::InitButtonEvent(nsMouseEvent &aEvent,
>                           GdkEventButton *aGdkEvent)
> {
>+    aEvent.pressure = aGdkEvent->axes?*aGdkEvent->axes:0;
>+
So the following is used somewhere in the patch:
gdk_event_get_axis ((GdkEvent*)aEvent, GDK_AXIS_PRESSURE, &pressure);
which at least looks better.
I'm not familiar with GdkEvent, but is axes really always pressure?
And what about the default value. Should it be 1?

the .idl interface (which should probably be called nsIDOMNSMouseEvent) and 
maybe .pressure in nsGUIEvent should be documented to inform user what
are the typical values. So is the range something between 0-1 or 0-100 or what?

Updated

9 years ago
Attachment #337281 - Flags: review?(Olli.Pettay) → review-
See https://bugs.webkit.org/show_bug.cgi?id=20458. I think making the pressure field a 0-1 float makes most sense. And we should definitely add the 'pressure' field to all mouse events.
I suppose the DOM attribute should be called 'mozPressure' though.

Comment 28

9 years ago
roc just pointed me at this, the webkit WIP is at
https://bugs.webkit.org/show_bug.cgi?id=20458

The approach we're looking at is to add pressure info into mouse events rather
than having a distinct event type.  We also give pressure a range [0..1] --
there's no point in having a 0..<some fixed non-1 value> as  most apps will
require a division to normalise before the pressure can be used anyway.

Another issue is that the browser should not be making a decision about what
the pressure ranges mean -- typically that's a preference made between both the
device and the application as a pair.  In practise this means that web app
making use of pressure information should provide the user with a pressure
preference.  The webkit implementation merely passes on the OS provided
pressure information on the assumption the user has already calibrated the
basic pressure information to what they are comfortable with.

One issue i have no real solution for (and why i asked roc whether moz was
working on this :D) is how to expose stylus eraser information seamlessly.  It
seems icky for it to be a flag, so i considered negative pressure, but that
also seems icky.

(And roc is correct -- ours should be webkitPressure, my bad)
I think we need to add also a new .initXXXEvent method.
Something which takes pressure as a parameter.
Maybe initMouseEventWithPressure(in DOMString typeArg, 
                                 in boolean canBubbleArg, 
                                 in boolean cancelableArg, 
                                 in AbstractView viewArg, 
                                 in long detailArg, 
                                 in long screenXArg, 
                                 in long screenYArg, 
                                 in long clientXArg, 
                                 in long clientYArg, 
                                 in boolean ctrlKeyArg, 
                                 in boolean altKeyArg, 
                                 in boolean shiftKeyArg, 
                                 in boolean metaKeyArg, 
                                 in unsigned short buttonArg, 
                                 in EventTarget relatedTargetArg,
                                 in double pressure)

We could also reuse initMouseEvent and add the last parameter as optional,
but it is a bit ugly to add new parameters to already standardized methods.
Do we really need that? I've never understood the need for the init methods.
(In reply to comment #29)
> I think we need to add also a new .initXXXEvent method.

I'd resist this.  I remember how much fun it was to discover the wantsUntrusted argument on addEventListener - which doesn't exist in the standard DOM model.  

Even with the "...WithPressure" qualifier, you're basically talking four, five new methods just to implement one new argument.  Better to have a separate setPressure method and make sure it can't do anything once the event's begun dispatching.
I'm a dunce - it'd be only one method, since only mouse events would have pressure.

roc:  The init methods are for creating a DOM event from scratch, setting all the basic parameters of the event before dispatching.  Rather handy when you want to fake an event - or create a custom event with a custom type, though fifteen arguments on one method is a bit much.
(In reply to comment #31)
> Even with the "...WithPressure" qualifier, you're basically talking four, five
> new methods just to implement one new argument. 
Er, what? It means just one very simple method implementation, because most of
the implementation can be forwarded to normal .initMouseEvent.
Or if we want to support .pressure also on dragEvents and mousescroll events, 
then 3 very simple methods. But I'm happy with .setMozPressure too, 
if it can be set only when event isn't being dispatched.

I think test frameworks really need to have some way to set pressure on
script generated events. So either .initXXX or .setMozPressure
(Assignee)

Comment 34

9 years ago
> So the following is used somewhere in the patch:
> gdk_event_get_axis ((GdkEvent*)aEvent, GDK_AXIS_PRESSURE, &pressure);
> which at least looks better.
> I'm not familiar with GdkEvent, but is axes really always pressure?

In this patch I'm using axis value to remember (previous) pressure value...

>+    event->axes = &pressure;
>     window->OnButtonReleaseEvent(widget, event);
>+    event->axes = NULL;

It looks hacky, but otherwise I need to add new argument to ::OnButton*Event methods...

> And what about the default value. Should it be 1?

pressure value range between 0-1. see Comment #c12

> 
> the .idl interface (which should probably be called nsIDOMNSMouseEvent) and 
> maybe .pressure in nsGUIEvent should be documented to inform user what
> are the typical values. So is the range something between 0-1 or 0-100 or what?
(In reply to comment #34)
> In this patch I'm using axis value to remember (previous) pressure value...
Well, if you do something like that, which is pretty hackish, please
document it clearly

> > And what about the default value. Should it be 1?
> 
> pressure value range between 0-1. see Comment #c12
Comment #12 doesn't say what the default value is.
The question is that what the value should be for example on my desktop 
machine, which certainly doesn't support pressure.
(Assignee)

Comment 36

9 years ago
> The question is that what the value should be for example on my desktop 
> machine, which certainly doesn't support pressure.

I think default value = 0, because it value when pressure not exists at all.
The same 0 you should get if Device extensions not enabled.
I think 1 would make a much better default pressure, or maybe 0.5.

Storing the pressure in the axis field seems like a really terrible idea. Add a new field for the pressure.
I agree, 1 or 0.5 could be better default value. Otherwise making a web app which
needs at least some pressure wouldn't work with normal mouse.
Summary: Exposed mouse event pressure → Expose mouse event pressure

Comment 39

9 years ago
Perhaps this should be in a different bug or a conversation in a newsgroup but it best ties into this bug so I'm starting here.

I am interested in implementing the Windows portion of this feature.  On Windows, there are two possible APIs that can be used for mouse event pressure:  Wintab API (used by Wacom) and Microsoft Ink.  Is there a preference for which API to support?

Here are some considerations:
* Wintab API has the widest range of support (XP, Tablet PC, Vista, etc.)
* Wintab SDK may have patent issues; see http://www.pointing.com/Wintab.html
  (Note that GIMP and other projects do use Wintab API)
* MS Ink is only supported on Vista and WinXP Tablet PC edition.

Are other people interested in exposing mouse pressure on Windows?
(Reporter)

Comment 40

9 years ago
over to roxama since he is working on it (or at least posted the last patch)
Assignee: doug.turner → romaxa
(Assignee)

Comment 41

9 years ago
Created attachment 349637 [details] [diff] [review]
Updated patch comm#25 + tested with N810

Updated according to comment #25.
Updated and tested on 810, added mLastMotionPressure member to remember last valid pressure value.
Attachment #335753 - Attachment is obsolete: true
Attachment #337281 - Attachment is obsolete: true
Attachment #337675 - Attachment is obsolete: true
Attachment #349637 - Flags: review?(roc)
Attachment #337281 - Flags: review?(roc)
(Assignee)

Comment 42

9 years ago
Comment on attachment 349637 [details] [diff] [review]
Updated patch comm#25 + tested with N810

>+    event.pressure = pressure ? pressure : mLastMotionPressure;
>+
Press and release events generate pressure value = 0, and I think it would be nice to have some last valid value from motion events.
Also possible here to use MAX motion events pressure value.

Btw: this patch supposed to be applied on top of https://bugzilla.mozilla.org/attachment.cgi?id=347874&action=edit
(Assignee)

Comment 43

9 years ago
nsIDOMMouseWithPressureEvent.idl - the same as in previous patches.

Comment 44

9 years ago
romaxa:  Have you had any thoughts on how to represent either the erase function that many tablets/styluses(styli?) have nowadays?
(Assignee)

Comment 45

9 years ago
oliver: "erase function"? can you explain it more clear?

Comment 46

9 years ago
(In reply to comment #45)
> oliver: "erase function"? can you explain it more clear?

Many tablet/styluses have an "erase" button -- eg, one end acts as the writing end, the other end acts as an eraser (much like a pencil :D ) -- I've never been able to come up with a nice way to expose this concept (i can't decide between an "erase" flag, or negative pressure, or whatever)
Could add a tool ID instead of just an erase flag, as some tablets have multiple tools and can distinguish between them.  I can also see some touch devices being able to distinguish between a finger or a pen being used where that info could be useful.
nsIDOMMouseWithPressureEvent.idl isn't in the patch.

I don't think we should have a separate nsDOMMouseWithPressureEvent class. All mouse events should expose pressure. nsIDOMMouseEvent is frozen, but we can create an nsIDOMNSMouseEvent that subclasses it and put pressure in there.
(Assignee)

Comment 49

9 years ago
Created attachment 349951 [details] [diff] [review]
nsIDOMNSEvent version.
Attachment #349637 - Attachment is obsolete: true
Attachment #349951 - Flags: review?(roc)
Attachment #349637 - Flags: review?(roc)
Roc said nsIDOMNSMouseEvent, not nsIDOMNSEvent ;)
(Assignee)

Comment 51

9 years ago
Created attachment 349977 [details] [diff] [review]
nsIDOMNSMouseEvent version.

>Roc said nsIDOMNSMouseEvent, not nsIDOMNSEvent ;)
oh, yep... 
Here is the right (I hope) version
Attachment #349951 - Attachment is obsolete: true
Attachment #349977 - Flags: review?(roc)
Attachment #349951 - Flags: review?(roc)
Should mousescroll (wheel) events support pressure? I think they may as well. In that case nsIDOMNSMouseEvent can inherit from nsIDOMMouseEvent.

Updated

9 years ago
Flags: wanted-fennec1.0+
(Assignee)

Comment 53

9 years ago
Created attachment 352002 [details] [diff] [review]
Pressure value for mouse scroll event

Added:
   DOM_CLASSINFO_MAP_BEGIN(MouseScrollEvent, nsIDOMMouseScrollEvent) 
     DOM_CLASSINFO_MAP_ENTRY(nsIDOMMouseScrollEvent) 
     DOM_CLASSINFO_MAP_ENTRY(nsIDOMMouseEvent) 
+    DOM_CLASSINFO_MAP_ENTRY(nsIDOMNSMouseEvent) 
     DOM_CLASSINFO_UI_EVENT_MAP_ENTRIES 
   DOM_CLASSINFO_MAP_END

To make nsIDOMNSMouseEvent available for nsIDOMMouseScrollEvent
Attachment #349977 - Attachment is obsolete: true
Attachment #352002 - Flags: review?(roc)
Attachment #349977 - Flags: review?(roc)
 class nsDOMMouseEvent : public nsIDOMMouseEvent,
-                        public nsDOMUIEvent
+                        public nsDOMUIEvent,
+                        public nsIDOMNSMouseEvent

This can be just "public nsDOMUIEvent, public nsIDOMNSMouseEvent".

+interface nsIDOMNSMouseEvent : nsISupports

This should inherit from nsIDOMMouseEvent. And it should have an initNSMouseEvent method I guess.

+  // Finger or touch pressure of event
+  float                 pressure;

Say that it ranges between 0.0 and 1.0.

+  readonly attribute float pressure;

Document that here too.

+    if (pressure)
+      mLastMotionPressure = pressure;

Why the 0 check here?
(Assignee)

Comment 55

9 years ago
Created attachment 352064 [details] [diff] [review]
Fixed comment #54

>+    if (pressure)
>+      mLastMotionPressure = pressure;
>Why the 0 check here?

Because gdk generates 0 values sometime, and always generate 0 value before mouse_release event, and it is not possible to get last valid pressure value on mouse release event.
Attachment #352002 - Attachment is obsolete: true
Attachment #352064 - Flags: review?(roc)
Attachment #352002 - Flags: review?(roc)
+    // Sometime gdk generate 0 pressure value between normal values
+    // We have to ignore that and use last valid value

So this is just a GDK bug we're working around?

So in nsWindow::OnButtonPressEvent we're not getting the GDK pressure at all, we just use the last pressure. This seems strange, does GDK really not have a pressure value for press events? That seems like the most important event to have that information for.
(Assignee)

Comment 57

9 years ago
I'm not sure where is original source for this problem (GDK, X-touch-lib or just hardware problem), but on my N810 it works in this way...

I don't know about other devices which has GTK + touch screen with pressure values... may be we can check there, or ask somebody else...

I will ask again some people who is working with gdk/xserver hopefully they have some meaningful answer on this question.

But in general I don't this that is current workaround is really bad.
(Assignee)

Comment 58

9 years ago
Also I think current implementation should works fine for devices with and without this problem with pressure values.
What about OnButtonPressEvent?
(Assignee)

Comment 60

9 years ago
(In reply to comment #59)
> What about OnButtonPressEvent?

Hmm, what is wrong with that?
@@ -2756,25 +2779,27 @@ nsWindow::OnButtonPressEvent(GtkWidget *
.......
+    gdouble pressure = 0;
+    gdk_event_get_axis ((GdkEvent*)aEvent, GDK_AXIS_PRESSURE, &pressure);
+    mLastMotionPressure = pressure;
.........
+    event.pressure = mLastMotionPressure;

value from get_axis should appear in NSEvent structure...

In theory it depends from low level implementation... if press event initialized when first touch happen, then pressure value should be 0, because at first moment touch surface area very small...

Otherwise press event should happen after some time when pressure value enough big ( > 0), but on n810 on that moment we already have a lot motion events with valid pressure value...
Also I have found that events order looks like:
motion
press
motion....motion
release
Comment on attachment 352064 [details] [diff] [review]
Fixed comment #54

OK, I misunderstood the patch.
Attachment #352064 - Flags: superreview+
Attachment #352064 - Flags: review?(roc)
Attachment #352064 - Flags: review?(Olli.Pettay)
Comment on attachment 352064 [details] [diff] [review]
Fixed comment #54

>+nsDOMMouseEvent::InitNSMouseEvent
...
>+  ((nsMouseEvent*)mEvent)->pressure = aPressure;
static_cast<nsMouseEvent_base*>->pressure = aPressure;


>+nsDOMMouseEvent::GetPressure(float* aPressure)
>+{
>+  NS_ENSURE_ARG_POINTER(aPressure);
>+  *aPressure = ((nsMouseEvent*)mEvent)->pressure;
*aPressure = static_cast<nsMouseEvent_base*>->pressure;

And didn't roc say earlier that the DOM attribute should be called .mozPressure, not
.pressure.

>   DOM_CLASSINFO_MAP_BEGIN(MouseEvent, nsIDOMMouseEvent)
>     DOM_CLASSINFO_MAP_ENTRY(nsIDOMMouseEvent)
>+    DOM_CLASSINFO_MAP_ENTRY(nsIDOMNSMouseEvent)
>     DOM_CLASSINFO_UI_EVENT_MAP_ENTRIES
>   DOM_CLASSINFO_MAP_END
> 
>   DOM_CLASSINFO_MAP_BEGIN(MouseScrollEvent, nsIDOMMouseScrollEvent)
>     DOM_CLASSINFO_MAP_ENTRY(nsIDOMMouseScrollEvent)
>     DOM_CLASSINFO_MAP_ENTRY(nsIDOMMouseEvent)
>+    DOM_CLASSINFO_MAP_ENTRY(nsIDOMNSMouseEvent)
>     DOM_CLASSINFO_UI_EVENT_MAP_ENTRIES
>   DOM_CLASSINFO_MAP_END
> 
>   DOM_CLASSINFO_MAP_BEGIN(DragEvent, nsIDOMDragEvent)
>     DOM_CLASSINFO_MAP_ENTRY(nsIDOMDragEvent)
>     DOM_CLASSINFO_UI_EVENT_MAP_ENTRIES
>   DOM_CLASSINFO_MAP_END
DragEvents are also MouseEvents. (I know DragEvent doesn't have nsIDOMMouseEvent atm,
but that will be fixed in a different bug.)

I still don't like it too much that the default value for pressure is 0, but I guess we can tweak that later.

With those and at least some simple tests for .initNSMouseEvent(...), r=me
Attachment #352064 - Flags: review?(Olli.Pettay) → review+
(In reply to comment #62)
> And didn't roc say earlier that the DOM attribute should be called
> .mozPressure, not .pressure.

I did, and then I forgot. So please fix that.
(Assignee)

Comment 64

9 years ago
Created attachment 352379 [details] [diff] [review]
Ready to check-in (fixed comment 62)
(Assignee)

Comment 65

9 years ago
Created attachment 352388 [details] [diff] [review]
Static cast + Drag event interface fixes
Attachment #352379 - Attachment is obsolete: true
The IDL file is missing from this patch?
(Assignee)

Comment 67

9 years ago
Created attachment 352417 [details] [diff] [review]
Version with idl file

> The IDL file is missing from this patch?
Yes.
Here is the version with idl file
Attachment #352388 - Attachment is obsolete: true
Looks good to me, check it in.
(Assignee)

Comment 69

9 years ago
Try server seems to build fine.
Pushed in:
http://hg.mozilla.org/mozilla-central/rev/3a8df4d8dec4
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED

Comment 70

9 years ago
Had to back this out due to persistent orange. Please try again when the tree is green.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 71

9 years ago
Let's try again:
http://hg.mozilla.org/mozilla-central/rev/ce5004d57908
Status: REOPENED → RESOLVED
Last Resolved: 9 years ago9 years ago
Resolution: --- → FIXED
(Assignee)

Updated

9 years ago
Depends on: 469756
Keywords: dev-doc-needed
This is now documented (as is MouseEvent in general, which had not previously been documented):

https://developer.mozilla.org/en/DOM/Event/UIEvent/MouseEvent

Mentioned on Fx4 for developers as well.
Keywords: dev-doc-needed → dev-doc-complete
Depends on: 879374
You need to log in before you can comment on or make changes to this bug.