Summary: | [GTK] Make native events code more consistent | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Carlos Garcia Campos <cgarcia> | ||||
Component: | WebKit2 | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | Keywords: | Gtk | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | PC | ||||||
OS: | Linux | ||||||
Attachments: |
|
Description
Carlos Garcia Campos
2011-05-05 01:49:10 PDT
Created attachment 92388 [details]
Patch
Comment on attachment 92388 [details] Patch 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. (In reply to comment #2) > (From update of attachment 92388 [details]) > 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? Because the default copy constructor doesn't know how to copy a GdkEvent. > > 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. Ok. > > Source/WebKit2/Shared/gtk/WebEventFactory.cpp:141 > > + switch (reinterpret_cast<const GdkEventScroll*>(event)->direction) { > > It's probably better to write event->scroll.direction. Ok. > > Source/WebKit2/Shared/gtk/WebEventFactory.cpp:175 > > + guint keyval = reinterpret_cast<const GdkEventKey*>(event)->keyval; > > Ditto. event->key.keyval. Ok. Comment on attachment 92388 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=92388&action=review >>> Source/WebKit2/Shared/NativeWebMouseEvent.h:52 >>> + NativeWebMouseEvent(const NativeWebMouseEvent&); >> >> Why is a copy constructor necessary now? > > Because the default copy constructor doesn't know how to copy a GdkEvent. Make sense! Might want to put a small note in the ChangeLog. Committed r85856: <http://trac.webkit.org/changeset/85856> |