764355 - Add new edgeui simple gesture for win8, and update tap gesture

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Comment 1

[Jim Mathies [:jimm]]

Attachment: patch v.1

Comment 2

[Jim Mathies [:jimm]]

Comment on attachment 632659 [details] [diff] [review]
patch v.1

This add a new gesture for win8 metro which fires when the user swipes inward across the display edge. It's used to invoke edge based ui in metro. I've also added a clickCount (similar to mouse click events) to the tap event to support new single and double tap events on Win8.

Comment 3

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 632659 [details] [diff] [review]
patch v.1

Review of attachment 632659 [details] [diff] [review]:
-----------------------------------------------------------------

I don't see anything here that gets this gesture from the OS and dispatches it.. Don't you need to add a new case in the switch statement at nsWinGesture::ProcessGestureMessage for it?

::: dom/interfaces/events/nsIDOMSimpleGestureEvent.idl
@@ +58,5 @@
>   * (XXX Not implemented on Mac)
>   *
> + * MozEdgeUIGesture - Generated when the user swipes the display to
> + * invoke edge ui.
> + * (XXX Win8 tablets only)

s/tablets/Metro ?

@@ +123,1 @@
>    void initSimpleGestureEvent(in DOMString typeArg,

hmm most others init*Event contains all possible fields for the event. although these functions are mostly just used for testing. I think it would be nice to stay consistent

::: widget/nsGUIEvent.h
@@ +430,5 @@
>  #define NS_SIMPLE_GESTURE_ROTATE_UPDATE  (NS_SIMPLE_GESTURE_EVENT_START+5)
>  #define NS_SIMPLE_GESTURE_ROTATE         (NS_SIMPLE_GESTURE_EVENT_START+6)
>  #define NS_SIMPLE_GESTURE_TAP            (NS_SIMPLE_GESTURE_EVENT_START+7)
>  #define NS_SIMPLE_GESTURE_PRESSTAP       (NS_SIMPLE_GESTURE_EVENT_START+8)
> +// win8 metro speicifc

typo. but this comment doesn't fit too well in this file where the other comments describe different sections of the file. You can just drop it, or maybe call the definition NS_SIMPLE_GESTURE_METRO_EDGEUI if you want.

Comment 4

[Jim Mathies [:jimm]]

Comment on attachment 632659 [details] [diff] [review]
patch v.1

(In reply to Felipe Gomes (:felipe) from comment #3)
> Comment on attachment 632659 [details] [diff] [review]
> patch v.1
> 
> Review of attachment 632659 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I don't see anything here that gets this gesture from the OS and dispatches
> it.. Don't you need to add a new case in the switch statement at
> nsWinGesture::ProcessGestureMessage for it?

This code is on elm and is only fired by the metro interface - 

http://mxr.mozilla.org/projects-central/source/elm/widget/windows/winrt/GestureInput.cpp

This bug is part of the process of moving all the elm code over to mc. The widget code hasn't landed yet, it will probably land last. 

> @@ +123,1 @@
> >    void initSimpleGestureEvent(in DOMString typeArg,
> 
> hmm most others init*Event contains all possible fields for the event.
> although these functions are mostly just used for testing. I think it would
> be nice to stay consistent

Wondered about that, will update. There are no optional params on these so I was trying to avoid updating all the callers. Not a big deal though.

Will fix the typos as well and repost.

Comment 5

[Jim Mathies [:jimm]]

Attachment: patch v.2

Comment 6

[Jim Mathies [:jimm]]

Comment on attachment 633169 [details] [diff] [review]
patch v.2

There weren't as many calls to that init function as I thought so it wasn't a big deal.

Comment 7

[Jim Mathies [:jimm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/542d031d6c3f

Thanks felipe!

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/542d031d6c3f