[webkit-reviews] review granted: [Bug 60258] [GTK] Make native events code more consistent : [Attachment 92388] Patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has granted Carlos Garcia Campos
<cgarcia at igalia.com>'s request for review:
Bug 60258: [GTK] Make native events code more consistent
https://bugs.webkit.org/show_bug.cgi?id=60258

Attachment 92388: Patch
https://bugs.webkit.org/attachment.cgi?id=92388&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=92388&action=review

Very nice cleanup, but I do not understand the addition of the copy
constructors.

> Source/WebKit2/Shared/NativeWebMouseEvent.h:52
> +    NativeWebMouseEvent(const NativeWebMouseEvent&);

Why is a copy constructor necessary now?

> Source/WebKit2/Shared/NativeWebWheelEvent.h:52
> +    NativeWebWheelEvent(const NativeWebWheelEvent&);

Ditto.

> Source/WebKit2/Shared/gtk/WebEventFactory.cpp:99
> +WebMouseEvent WebEventFactory::createWebMouseEvent(const GdkEvent *event,
int currentClickCount)

The asterisk should move to the left here.

> Source/WebKit2/Shared/gtk/WebEventFactory.cpp:141
> +    switch (reinterpret_cast<const GdkEventScroll*>(event)->direction) {

It's probably better to write event->scroll.direction.

> Source/WebKit2/Shared/gtk/WebEventFactory.cpp:175
> +    guint keyval = reinterpret_cast<const GdkEventKey*>(event)->keyval;

Ditto. event->key.keyval.