I have added NativeWebMouseEvent and NativeWebWheelEvent for WebKit2 EFL port.
Created attachment 95856 [details] Patch
Comment on attachment 95856 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95856&action=review I know nothing about WebKit2, but I can comment about some style issues. > Source/WebKit2/Shared/efl/NativeWebMouseEventEfl.cpp:37 > +NativeWebMouseEvent::NativeWebMouseEvent(const Evas_Event_Mouse_Down* event, const Evas_Point* position) > + : WebMouseEvent(WebEventFactory::createWebMouseEvent(event, position)) > + , m_nativeEvent(event) > +{ > +} notImplemented(); > Source/WebKit2/Shared/efl/NativeWebMouseEventEfl.cpp:43 > +NativeWebMouseEvent::NativeWebMouseEvent(const Evas_Event_Mouse_Up* event, const Evas_Point* position) > + : WebMouseEvent(WebEventFactory::createWebMouseEvent(event, position)) > + , m_nativeEvent(event) > +{ > +} notImplemented(); > Source/WebKit2/Shared/efl/NativeWebMouseEventEfl.cpp:49 > +NativeWebMouseEvent::NativeWebMouseEvent(const Evas_Event_Mouse_Move* event, const Evas_Point* position) > + : WebMouseEvent(WebEventFactory::createWebMouseEvent(event, position)) > + , m_nativeEvent(event) > +{ > +} notImplemented(); > Source/WebKit2/Shared/efl/NativeWebWheelEventEfl.cpp:37 > +NativeWebWheelEvent::NativeWebWheelEvent(const Evas_Event_Mouse_Wheel* event, const Evas_Point* position) > + : WebWheelEvent(WebEventFactory::createWebWheelEvent(event, position)) > + , m_nativeEvent(event) > +{ > +} notImplemented(); > Source/WebKit2/Shared/efl/WebEventFactory.cpp:52 > + return (WebEvent::Modifiers)result; Use C++-style cast. > Source/WebKit2/Shared/efl/WebEventFactory.h:43 > + Whereis the #endif?
Created attachment 96178 [details] Patch
(In reply to comment #2) > (From update of attachment 95856 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=95856&action=review > > I know nothing about WebKit2, but I can comment about some style issues. > > > Source/WebKit2/Shared/efl/NativeWebMouseEventEfl.cpp:37 > > +NativeWebMouseEvent::NativeWebMouseEvent(const Evas_Event_Mouse_Down* event, const Evas_Point* position) > > + : WebMouseEvent(WebEventFactory::createWebMouseEvent(event, position)) > > + , m_nativeEvent(event) > > +{ > > +} > > notImplemented(); > Those are all codes for NativeWebMouseEvent::NativeWebMouseEvent. There is nothing to implement more. > > Source/WebKit2/Shared/efl/NativeWebMouseEventEfl.cpp:43 > > +NativeWebMouseEvent::NativeWebMouseEvent(const Evas_Event_Mouse_Up* event, const Evas_Point* position) > > + : WebMouseEvent(WebEventFactory::createWebMouseEvent(event, position)) > > + , m_nativeEvent(event) > > +{ > > +} > > notImplemented(); > Ditto. > > Source/WebKit2/Shared/efl/NativeWebMouseEventEfl.cpp:49 > > +NativeWebMouseEvent::NativeWebMouseEvent(const Evas_Event_Mouse_Move* event, const Evas_Point* position) > > + : WebMouseEvent(WebEventFactory::createWebMouseEvent(event, position)) > > + , m_nativeEvent(event) > > +{ > > +} > > notImplemented(); > Ditto. > > Source/WebKit2/Shared/efl/NativeWebWheelEventEfl.cpp:37 > > +NativeWebWheelEvent::NativeWebWheelEvent(const Evas_Event_Mouse_Wheel* event, const Evas_Point* position) > > + : WebWheelEvent(WebEventFactory::createWebWheelEvent(event, position)) > > + , m_nativeEvent(event) > > +{ > > +} > > notImplemented(); > Ditto. > > Source/WebKit2/Shared/efl/WebEventFactory.cpp:52 > > + return (WebEvent::Modifiers)result; > > Use C++-style cast. > I've changed code to use C++ style cast. > > Source/WebKit2/Shared/efl/WebEventFactory.h:43 > > + > > Whereis the #endif? The #endif exists in the line 44 :)
Comment on attachment 96178 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=96178&action=review Patch looks OK, but I don't know much about WebKit2 to properly review it. > Source/WebKit2/ChangeLog:8 > + [EFL][WK2] add WebKit2 EFL port's NativeWebMouseEvent and NativeWebWheelEvent > + https://bugs.webkit.org/show_bug.cgi?id=61993 > + > + Add WebKit2 EFL port's NativeWebMouseEvent and NativeWebWheelEvent ChangeLog entry says the same thing twice.
Created attachment 96343 [details] Patch
(In reply to comment #5) > (From update of attachment 96178 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=96178&action=review > > Patch looks OK, but I don't know much about WebKit2 to properly review it. > > > Source/WebKit2/ChangeLog:8 > > + [EFL][WK2] add WebKit2 EFL port's NativeWebMouseEvent and NativeWebWheelEvent > > + https://bugs.webkit.org/show_bug.cgi?id=61993 > > + > > + Add WebKit2 EFL port's NativeWebMouseEvent and NativeWebWheelEvent > > ChangeLog entry says the same thing twice. I've changed the ChangeLog. I'm always thankful for your review :)
(In reply to comment #5) > (From update of attachment 96178 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=96178&action=review > > Patch looks OK, but I don't know much about WebKit2 to properly review it. LGTM. However, I don't know who can review this patch as well. Let's find proper reviewers.
Created attachment 96376 [details] Patch
Created attachment 96732 [details] Patch
(In reply to comment #10) > Created an attachment (id=96732) [details] > Patch I've added NativeWebKeyboardEvent.
Comment on attachment 96732 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=96732&action=review > Source/WebKit2/ChangeLog:7 > + Missing summary for Patch > Source/WebKit2/Shared/efl/WebEventFactory.cpp:60 > + if (button == 1) I think it is better to use constant instead of "1" > Source/WebKit2/Shared/efl/WebEventFactory.cpp:62 > + if (button == 2) ditto.
Created attachment 97105 [details] Patch
(In reply to comment #12) > (From update of attachment 96732 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=96732&action=review > > > Source/WebKit2/ChangeLog:7 > > + > > Missing summary for Patch Done. > > > Source/WebKit2/Shared/efl/WebEventFactory.cpp:60 > > + if (button == 1) > > I think it is better to use constant instead of "1" > > > Source/WebKit2/Shared/efl/WebEventFactory.cpp:62 > > + if (button == 2) > > ditto. Done. I added enum for that.
Comment on attachment 97105 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=97105&action=review Informal R+. > Source/WebKit2/ChangeLog:8 > + [EFL][WK2] add WebKit2 EFL port's NativeWebMouseEvent, NativeWebWheelEvent and NativeWebKeyboardEvent > + https://bugs.webkit.org/show_bug.cgi?id=61993 > + > + add native mouse and keyboard event classes to convert EFL's events to NativeWebEvent. Start sentences with uppercase letters.
Comment on attachment 97105 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=97105&action=review >> Source/WebKit2/ChangeLog:8 >> + add native mouse and keyboard event classes to convert EFL's events to NativeWebEvent. > > Start sentences with uppercase letters. Add* > Source/WebKit2/Shared/efl/WebEventFactory.cpp:46 > +enum { > + LeftButton = 1, > + MiddleButton = 2, > + RightButton = 3 > +}; Is this always the case for all mouse types?
Created attachment 97217 [details] Patch
(In reply to comment #16) > (From update of attachment 97105 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=97105&action=review > > >> Source/WebKit2/ChangeLog:8 > >> + add native mouse and keyboard event classes to convert EFL's events to NativeWebEvent. > > > > Start sentences with uppercase letters. > > Add* > Done. > > Source/WebKit2/Shared/efl/WebEventFactory.cpp:46 > > +enum { > > + LeftButton = 1, > > + MiddleButton = 2, > > + RightButton = 3 > > +}; > > Is this always the case for all mouse types? I am not sure about that, but I think I can't support more because WebMouseEvent supports only those three buttons now.
Comment on attachment 97217 [details] Patch Clearing flags on attachment: 97217 Committed r89211: <http://trac.webkit.org/changeset/89211>
All reviewed patches have been landed. Closing bug.