Bug 88633

Summary: [EFL] Extract CursorMap from WidgetEfl.cpp.
Product: WebKit Reporter: Ryuan Choi <ryuan.choi>
Component: WebKit EFLAssignee: Ryuan Choi <ryuan.choi>
Status: RESOLVED FIXED    
Severity: Normal CC: gyuyoung.kim, gyuyoung.kim, lucas.de.marchi, rakuco, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 88803    
Attachments:
Description Flags
Patch
none
Patch none

Description Ryuan Choi 2012-06-08 01:54:13 PDT
Because WebProcess can not control cursor,
setCursor should be passed to UIProcess via ChromeClient.

As a first step,
Ecore_X_Cursor should be moved out of Widget to call it in WebKit/efl and WebKit2/efl.
Comment 1 Ryuan Choi 2012-06-08 02:25:49 PDT
Created attachment 146519 [details]
Patch
Comment 2 Ryuan Choi 2012-06-08 02:27:47 PDT
(In reply to comment #0)
> Because WebProcess can not control cursor,
> setCursor should be passed to UIProcess via ChromeClient.
> 
> As a first step,
> Ecore_X_Cursor should be moved out of Widget to call it in WebKit/efl and WebKit2/efl.

s/Ecore_X_Cursor/CursorMap

I will move Ecore_X_Cursor and other cursor related implementations as next steps.
Comment 3 Chris Dumez 2012-06-09 04:57:45 PDT
Comment on attachment 146519 [details]
Patch

LGTM.
Comment 4 Gyuyoung Kim 2012-06-10 20:32:20 PDT
Comment on attachment 146519 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=146519&action=review

> Source/WebCore/ChangeLog:9
> +        Extract it and expose getEcoreCursor.

s/Extract/extract/g

> Source/WebCore/ChangeLog:11
> +        No new tests; refactoring only.

s/;/,/g

> Source/WebCore/platform/efl/EflScreenUtilities.cpp:52
> +    m_cursorStringMap.set("cursor/pointer", ECORE_X_CURSOR_LEFT_PTR);

I think it is better to list these cursor map up by alphabetical order.

> Source/WebCore/platform/efl/WidgetEfl.cpp:-43
> -

Do not remove code unrelated to this patch.
Comment 5 Ryuan Choi 2012-06-10 21:13:22 PDT
Created attachment 146773 [details]
Patch
Comment 6 Ryuan Choi 2012-06-10 21:17:43 PDT
(In reply to comment #4)
> (From update of attachment 146519 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=146519&action=review
> 
> > Source/WebCore/ChangeLog:9
> > +        Extract it and expose getEcoreCursor.
> 
> s/Extract/extract/g
Done.

> 
> > Source/WebCore/ChangeLog:11
> > +        No new tests; refactoring only.
> 
> s/;/,/g
Done.

> 
> > Source/WebCore/platform/efl/EflScreenUtilities.cpp:52
> > +    m_cursorStringMap.set("cursor/pointer", ECORE_X_CURSOR_LEFT_PTR);
> 
> I think it is better to list these cursor map up by alphabetical order.
It is ordered to match with Cursor::Type in Source/WebCore/platform/Cursor.h.
IMHO, keeping current order is easier to detect any changes of cursor.

> 
> > Source/WebCore/platform/efl/WidgetEfl.cpp:-43
> > -
> 
> Do not remove code unrelated to this patch.
Done.
Comment 7 Gyuyoung Kim 2012-06-10 22:03:32 PDT
Ok, Looks good to me now.
Comment 8 WebKit Review Bot 2012-06-11 00:41:35 PDT
Comment on attachment 146773 [details]
Patch

Clearing flags on attachment: 146773

Committed r119958: <http://trac.webkit.org/changeset/119958>
Comment 9 WebKit Review Bot 2012-06-11 00:41:41 PDT
All reviewed patches have been landed.  Closing bug.