Now, ewebkit changed mouse cursor when the type of cursor is changed. But ewebkit does not know whether other widgets such as elm_entry change the cursor. So we should restore the cursor when mouser enters on webview not to show wrong cursor.
Created attachment 226581 [details] suggestion
Comment on attachment 226581 [details] suggestion View in context: https://bugs.webkit.org/attachment.cgi?id=226581&action=review > Source/WebKit2/UIProcess/API/efl/EwkView.cpp:1165 > + EwkViewEventHandler<EVAS_CALLBACK_MOUSE_IN>::subscribe(evasObject); Why don't you follow the alphabetic order?
Created attachment 226871 [details] Patch
(In reply to comment #2) > (From update of attachment 226581 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=226581&action=review > > > Source/WebKit2/UIProcess/API/efl/EwkView.cpp:1165 > > + EwkViewEventHandler<EVAS_CALLBACK_MOUSE_IN>::subscribe(evasObject); > > Why don't you follow the alphabetic order? Fixed order.
Looks good to me.
Comment on attachment 226871 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=226871&action=review > Source/WebKit2/UIProcess/API/efl/EwkView.cpp:498 > + m_useCustomCursor = true; If possible, I prefer not to keep a member variable for boolean flag. Can't we pass this true/false to updateCursor() as an argument ? > Source/WebKit2/UIProcess/API/efl/EwkView.h:280 > + } m_cursorIdentifier; // This is an address, do not free it. Hmm, is m_cursorIdentifier still an address ?
Created attachment 226899 [details] Patch
(In reply to comment #6) > (From update of attachment 226871 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=226871&action=review > > > Source/WebKit2/UIProcess/API/efl/EwkView.cpp:498 > > + m_useCustomCursor = true; > > If possible, I prefer not to keep a member variable for boolean flag. Can't we pass this true/false to updateCursor() as an argument ? > Unfortunately, we should save the state of cursor for the MOUSE_IN callback. > > Source/WebKit2/UIProcess/API/efl/EwkView.h:280 > > + } m_cursorIdentifier; // This is an address, do not free it. > > Hmm, is m_cursorIdentifier still an address ? Right, I removed the comment.
Comment on attachment 226871 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=226871&action=review >>> Source/WebKit2/UIProcess/API/efl/EwkView.cpp:498 >>> + m_useCustomCursor = true; >> >> If possible, I prefer not to keep a member variable for boolean flag. Can't we pass this true/false to updateCursor() as an argument ? > > Unfortunately, we should save the state of cursor for the MOUSE_IN callback. It looks m_useCustomCursor is only used by updateCursor(). Could you let me know how is this used by the MOUSE_IN callback ?
(In reply to comment #9) > (From update of attachment 226871 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=226871&action=review > > >>> Source/WebKit2/UIProcess/API/efl/EwkView.cpp:498 > >>> + m_useCustomCursor = true; > >> > >> If possible, I prefer not to keep a member variable for boolean flag. Can't we pass this true/false to updateCursor() as an argument ? > > > > Unfortunately, we should save the state of cursor for the MOUSE_IN callback. > > It looks m_useCustomCursor is only used by updateCursor(). Could you let me know how is this used by the MOUSE_IN callback ? Yes and EwkViewEventHandler<EVAS_CALLBACK_MOUSE_IN>::handleEvent() and setCursor() call updateCursor(). Although we can pass the true/false to updateCursor(), we should keep the any member variable to know which type of cursor(group or image) we will restore in the MOUSE_IN callback.
(In reply to comment #10) > (In reply to comment #9) > > (From update of attachment 226871 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=226871&action=review > > > > >>> Source/WebKit2/UIProcess/API/efl/EwkView.cpp:498 > > >>> + m_useCustomCursor = true; > > >> > > >> If possible, I prefer not to keep a member variable for boolean flag. Can't we pass this true/false to updateCursor() as an argument ? > > > > > > Unfortunately, we should save the state of cursor for the MOUSE_IN callback. > > > > It looks m_useCustomCursor is only used by updateCursor(). Could you let me know how is this used by the MOUSE_IN callback ? > > Yes and EwkViewEventHandler<EVAS_CALLBACK_MOUSE_IN>::handleEvent() and setCursor() call updateCursor(). I can't find "EwkViewEventHandler<EVAS_CALLBACK_MOUSE_IN>::handleEvent()" in EwkView.cpp. Is it supported by WebKit EFL trunk now ? Please let me know where it is implemented.
Comment on attachment 226871 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=226871&action=review >>>>>> Source/WebKit2/UIProcess/API/efl/EwkView.cpp:498 >>>>>> + m_useCustomCursor = true; >>>>> >>>>> If possible, I prefer not to keep a member variable for boolean flag. Can't we pass this true/false to updateCursor() as an argument ? >>>> >>>> Unfortunately, we should save the state of cursor for the MOUSE_IN callback. >>> >>> It looks m_useCustomCursor is only used by updateCursor(). Could you let me know how is this used by the MOUSE_IN callback ? >> >> Yes and EwkViewEventHandler<EVAS_CALLBACK_MOUSE_IN>::handleEvent() and setCursor() call updateCursor(). >> >> Although we can pass the true/false to updateCursor(), we should keep the any member variable to know which type of cursor(group or image) we will restore in the MOUSE_IN callback. > > I can't find "EwkViewEventHandler<EVAS_CALLBACK_MOUSE_IN>::handleEvent()" in EwkView.cpp. Is it supported by WebKit EFL trunk now ? Please let me know where it is implemented. I missed to see "EwkViewEventHandler<EVAS_CALLBACK_MOUSE_IN>::handleEvent()" in this patch. Sorry for noise.
Comment on attachment 226899 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=226899&action=review LGTM now. > Source/WebKit2/UIProcess/API/efl/EwkView.cpp:492 > + if (cursor.image()) { Trivial recommendation : How about using isNull() explicitly as below ? !cursor.image()->isNull()
Comment on attachment 226899 [details] Patch Clearing flags on attachment: 226899 Committed r165730: <http://trac.webkit.org/changeset/165730>
All reviewed patches have been landed. Closing bug.