Last Comment Bug 764355 - Add new edgeui simple gesture for win8, and update tap gesture
: Add new edgeui simple gesture for win8, and update tap gesture
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Events (show other bugs)
: Trunk
: x86_64 Windows 8.1
: -- normal (vote)
: mozilla16
Assigned To: Jim Mathies [:jimm]
:
Mentors:
Depends on:
Blocks: elm-merge
  Show dependency treegraph
 
Reported: 2012-06-13 06:13 PDT by Jim Mathies [:jimm]
Modified: 2014-07-24 11:06 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch v.1 (19.24 KB, patch)
2012-06-13 06:13 PDT, Jim Mathies [:jimm]
no flags Details | Diff | Splinter Review
patch v.2 (24.30 KB, patch)
2012-06-14 09:55 PDT, Jim Mathies [:jimm]
felipc: review+
Details | Diff | Splinter Review

Description Jim Mathies [:jimm] 2012-06-13 06:13:12 PDT

    
Comment 1 Jim Mathies [:jimm] 2012-06-13 06:13:33 PDT
Created attachment 632659 [details] [diff] [review]
patch v.1
Comment 2 Jim Mathies [:jimm] 2012-06-13 06:17:44 PDT
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 me!) 2012-06-13 14:26:29 PDT
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] 2012-06-13 14:39:00 PDT
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] 2012-06-14 09:55:31 PDT
Created attachment 633169 [details] [diff] [review]
patch v.2
Comment 6 Jim Mathies [:jimm] 2012-06-14 09:57:32 PDT
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] 2012-06-14 10:42:11 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/542d031d6c3f

Thanks felipe!
Comment 8 Ed Morley [:emorley] 2012-06-15 05:59:27 PDT
https://hg.mozilla.org/mozilla-central/rev/542d031d6c3f

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