Bug 150179 – Add magnify and rotate gesture event support for Mac WebKit2

• Status: RESOLVED
• Resolution: FIXED
• Component: UI Events

[Tim Horton]

Created attachment 263176 [details]
patch

Add magnify and rotate gesture event support for Mac WebKit2

[WebKit Commit Bot]

Attachment 263176 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/mac/ViewGestureController.h:103:  The parameter name "event" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
ERROR: Source/JavaScriptCore/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
ERROR: Source/WebKit2/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
ERROR: Source/WebKit2/Shared/mac/NativeWebGestureEventMac.mm:70:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebKit2/Shared/mac/NativeWebGestureEventMac.mm:64:  Comma should be at the beginning of the line in a member initialization list.  [whitespace/init] [4]
ERROR: Source/WebKit2/Shared/mac/NativeWebGestureEventMac.mm:65:  Comma should be at the beginning of the line in a member initialization list.  [whitespace/init] [4]
ERROR: Source/WebKit2/Shared/mac/NativeWebGestureEventMac.mm:67:  Comma should be at the beginning of the line in a member initialization list.  [whitespace/init] [4]
ERROR: Source/WebCore/ChangeLog:3:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
ERROR: Source/WebKit/mac/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 10 in 43 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Tim Horton]

Created attachment 263311 [details]
Patch

[WebKit Commit Bot]

Attachment 263311 [details] did not pass style-queue:


ERROR: Source/WebKit2/Shared/mac/NativeWebGestureEventMac.mm:70:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebKit2/Shared/mac/NativeWebGestureEventMac.mm:64:  Comma should be at the beginning of the line in a member initialization list.  [whitespace/init] [4]
ERROR: Source/WebKit2/Shared/mac/NativeWebGestureEventMac.mm:65:  Comma should be at the beginning of the line in a member initialization list.  [whitespace/init] [4]
ERROR: Source/WebKit2/Shared/mac/NativeWebGestureEventMac.mm:67:  Comma should be at the beginning of the line in a member initialization list.  [whitespace/init] [4]
Total errors found: 4 in 44 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Tim Horton]

Created attachment 263337 [details]
Patch

[WebKit Commit Bot]

Attachment 263337 [details] did not pass style-queue:


ERROR: Source/WebKit2/Shared/mac/NativeWebGestureEventMac.mm:70:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebKit2/Shared/mac/NativeWebGestureEventMac.mm:64:  Comma should be at the beginning of the line in a member initialization list.  [whitespace/init] [4]
ERROR: Source/WebKit2/Shared/mac/NativeWebGestureEventMac.mm:65:  Comma should be at the beginning of the line in a member initialization list.  [whitespace/init] [4]
ERROR: Source/WebKit2/Shared/mac/NativeWebGestureEventMac.mm:67:  Comma should be at the beginning of the line in a member initialization list.  [whitespace/init] [4]
Total errors found: 4 in 44 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Tim Horton]

Created attachment 263341 [details]
Patch

[WebKit Commit Bot]

Attachment 263341 [details] did not pass style-queue:


ERROR: Source/WebKit2/Shared/mac/NativeWebGestureEventMac.mm:70:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebKit2/Shared/mac/NativeWebGestureEventMac.mm:64:  Comma should be at the beginning of the line in a member initialization list.  [whitespace/init] [4]
ERROR: Source/WebKit2/Shared/mac/NativeWebGestureEventMac.mm:65:  Comma should be at the beginning of the line in a member initialization list.  [whitespace/init] [4]
ERROR: Source/WebKit2/Shared/mac/NativeWebGestureEventMac.mm:67:  Comma should be at the beginning of the line in a member initialization list.  [whitespace/init] [4]
Total errors found: 4 in 42 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Darin Adler]

Comment on attachment 263341 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=263341&action=review

> Source/WebCore/page/EventHandler.h:68
>  #if ENABLE(IOS_TOUCH_EVENTS)
> -#include <wtf/HashSet.h>
>  #include <wtf/Vector.h>
>  #endif
>  
> +#if ENABLE(IOS_TOUCH_EVENTS) || ENABLE(MAC_GESTURE_EVENTS)
> +#include <wtf/HashSet.h>
> +#endif

These kinds of generic WTF includes should probably be included without an #if. Having a precise include set for each combination doesn’t seem helpful.

> Source/WebKit2/Shared/NativeWebGestureEvent.h:38
> +class NativeWebGestureEvent : public WebGestureEvent {

final?

> Source/WebKit2/Shared/NativeWebGestureEvent.h:42
> +    explicit NativeWebGestureEvent(NSEvent *, NSView *);
> +
> +    NSEvent* nativeEvent() const { return m_nativeEvent.get(); }

NSEvent * formatting inconsistent here.

> Source/WebKit2/Shared/mac/WebGestureEvent.h:68
> +    WebCore::IntPoint m_position;

int, not double?

> Source/WebKit2/Shared/mac/WebGestureEvent.h:70
> +    float m_gestureScale;
> +    float m_gestureRotation;

float, not double?

> Source/WebKit2/WebProcess/WebPage/EventDispatcher.cpp:150
> +    RefPtr<EventDispatcher> eventDispatcher(this);

I normally like "= this" instead of "(this)"; not sure why. I guess that "()" is my initialization syntax of last resort. I even prefer the "{}" since it’s pickier about types and less likely to look like a function declaration.

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:2195
> +        CurrentEvent currentEvent(gestureEvent);
> +
> +        handled = handleGestureEvent(gestureEvent, m_page.get());

No need for the blank line. It’s not like local variables need to be separated at the top. Reads better without it.

[Tim Horton]

(In reply to comment #8)
> > Source/WebKit2/Shared/mac/WebGestureEvent.h:68
> > +    WebCore::IntPoint m_position;
> 
> int, not double?

Matches all the other events.

> > Source/WebKit2/Shared/mac/WebGestureEvent.h:70
> > +    float m_gestureScale;
> > +    float m_gestureRotation;
> 
> float, not double?

Matches WebTouchEvent.

[Tim Horton]

http://trac.webkit.org/changeset/191299

[Tim Horton]

http://trac.webkit.org/changeset/191305