Bug 61993

Summary: [EFL][WK2] add WebKit2 EFL port's NativeWebMouseEvent, NativeWebWheelEvent and NativeWebKeyboardEvent
Product: WebKit Reporter: Eunmi Lee <enmi.lee>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: antognolli+webkit, gyuyoung.kim, kenneth, leandro, ryuan.choi, sangseok.lim, tonikitoo, webkit.review.bot, youngtaeck.song
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 62451    
Bug Blocks: 61838    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Eunmi Lee
Reported 2011-06-02 23:24:15 PDT
I have added NativeWebMouseEvent and NativeWebWheelEvent for WebKit2 EFL port.
Attachments
Patch (16.89 KB, patch)
2011-06-02 23:36 PDT, Eunmi Lee 		no flags
DetailsFormatted DiffDiff
Patch (16.86 KB, patch)
2011-06-06 19:14 PDT, Eunmi Lee 		no flags
DetailsFormatted DiffDiff
Patch (16.78 KB, patch)
2011-06-07 17:42 PDT, Eunmi Lee 		no flags
DetailsFormatted DiffDiff
Patch (16.77 KB, patch)
2011-06-07 21:50 PDT, Eunmi Lee 		no flags
DetailsFormatted DiffDiff
Patch (22.45 KB, patch)
2011-06-10 04:39 PDT, Eunmi Lee 		no flags
DetailsFormatted DiffDiff
Patch (22.65 KB, patch)
2011-06-14 06:23 PDT, Eunmi Lee 		no flags
DetailsFormatted DiffDiff
Patch (22.65 KB, patch)
2011-06-14 18:51 PDT, Eunmi Lee 		no flags
DetailsFormatted DiffDiff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.
Eunmi Lee
Comment 1 2011-06-02 23:36:55 PDT
Created attachment 95856 [details] Patch
Leandro Pereira
Comment 2 2011-06-03 10:14:03 PDT
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?
Eunmi Lee
Comment 3 2011-06-06 19:14:00 PDT
Created attachment 96178 [details] Patch
Eunmi Lee
Comment 4 2011-06-06 19:19:34 PDT
(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 :)
Leandro Pereira
Comment 5 2011-06-07 07:34:48 PDT
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.
Eunmi Lee
Comment 6 2011-06-07 17:42:48 PDT
Created attachment 96343 [details] Patch
Eunmi Lee
Comment 7 2011-06-07 17:46:25 PDT
(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 :)
Gyuyoung Kim
Comment 8 2011-06-07 18:50:29 PDT
(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.
Eunmi Lee
Comment 9 2011-06-07 21:50:04 PDT
Created attachment 96376 [details] Patch
Eunmi Lee
Comment 10 2011-06-10 04:39:37 PDT
Created attachment 96732 [details] Patch
Eunmi Lee
Comment 11 2011-06-10 04:40:16 PDT
(In reply to comment #10) > Created an attachment (id=96732) [details] > Patch I've added NativeWebKeyboardEvent.
Gyuyoung Kim
Comment 12 2011-06-13 20:06:52 PDT
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.
Eunmi Lee
Comment 13 2011-06-14 06:23:19 PDT
Created attachment 97105 [details] Patch
Eunmi Lee
Comment 14 2011-06-14 06:31:00 PDT
(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.
Leandro Pereira
Comment 15 2011-06-14 08:51:35 PDT
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.
Kenneth Rohde Christiansen
Comment 16 2011-06-14 15:26:00 PDT
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?
Eunmi Lee
Comment 17 2011-06-14 18:51:10 PDT
Created attachment 97217 [details] Patch
Eunmi Lee
Comment 18 2011-06-14 18:56:11 PDT
(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.
WebKit Review Bot
Comment 19 2011-06-18 20:44:15 PDT
Comment on attachment 97217 [details] Patch Clearing flags on attachment: 97217 Committed r89211: <http://trac.webkit.org/changeset/89211>
WebKit Review Bot
Comment 20 2011-06-18 20:44:21 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.