Bug 112403 – [EFL] Provide default cursor groups as cursor.edc

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Jinwoo Song]

We can use the CSS cursor images in the WebCore/Resources/*Cursor.png images by providing a cursor.edc.

[Jinwoo Song]

Created attachment 193234 [details]
Patch

[Chris Dumez]

Comment on attachment 193234 [details]
Patch

LGTM.

[Ryuan Choi]

Comment on attachment 193234 [details]
Patch

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

> Source/WebCore/platform/efl/DefaultTheme/widget/cursor/cursor.edc:30
> +         image: "../../../Resources/linkCursor.png" COMP;

Can we pass CursorImage_DIR instead of duplication?

[Gyuyoung Kim]

Comment on attachment 193234 [details]
Patch

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

> Source/WebCore/platform/efl/DefaultTheme/CMakeLists.txt:123
> +            ${CursorImage_DIR}/westResizeCursor.png

Why zoomInCursor.png and zoomOutCursor.png aren't included ?

> Source/WebCore/platform/efl/DefaultTheme/widget/cursor/cursor.edc:33
> +

I prefer to align with other .edc file style. It looks this line is not needed.

> Source/WebCore/platform/efl/DefaultTheme/widget/cursor/cursor.edc:48
> +  group {

Wrong indentation ?

[Jinwoo Song]

Comment on attachment 193234 [details]
Patch

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

>> Source/WebCore/platform/efl/DefaultTheme/CMakeLists.txt:123
>> +            ${CursorImage_DIR}/westResizeCursor.png
> 
> Why zoomInCursor.png and zoomOutCursor.png aren't included ?

It seems those cursors are not supported in WebKit. Chrome browser do not show them either.
https://developer.mozilla.org/en-US/docs/CSS/cursor

>> Source/WebCore/platform/efl/DefaultTheme/widget/cursor/cursor.edc:30
>> +         image: "../../../Resources/linkCursor.png" COMP;
> 
> Can we pass CursorImage_DIR instead of duplication?

We can do it. I'll fix it.

>> Source/WebCore/platform/efl/DefaultTheme/widget/cursor/cursor.edc:33
>> +
> 
> I prefer to align with other .edc file style. It looks this line is not needed.

Okay, I'll remove unneeded lines.

>> Source/WebCore/platform/efl/DefaultTheme/widget/cursor/cursor.edc:48
>> +  group {
> 
> Wrong indentation ?

Copy&paste mistake. I'll fix it.

[Jinwoo Song]

Created attachment 193270 [details]
Patch

Applied the comments by Ryuan and Gyuyoung.

[Gyuyoung Kim]

Comment on attachment 193270 [details]
Patch

LGTM now.

[WebKit Review Bot]

Comment on attachment 193270 [details]
Patch

Clearing flags on attachment: 193270

Committed r146007: <http://trac.webkit.org/changeset/146007>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.