Bug 112403 - [EFL] Provide default cursor groups as cursor.edc
Summary: [EFL] Provide default cursor groups as cursor.edc
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jinwoo Song
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-14 22:09 PDT by Jinwoo Song
Modified: 2013-03-17 03:46 PDT (History)
5 users (show)

See Also:


Attachments
Patch (18.66 KB, patch)
2013-03-14 22:33 PDT, Jinwoo Song
no flags Details | Formatted Diff | Diff
Patch (19.07 KB, patch)
2013-03-15 03:12 PDT, Jinwoo Song
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jinwoo Song 2013-03-14 22:09:34 PDT
We can use the CSS cursor images in the WebCore/Resources/*Cursor.png images by providing a cursor.edc.
Comment 1 Jinwoo Song 2013-03-14 22:33:48 PDT
Created attachment 193234 [details]
Patch
Comment 2 Chris Dumez 2013-03-15 00:01:32 PDT
Comment on attachment 193234 [details]
Patch

LGTM.
Comment 3 Ryuan Choi 2013-03-15 00:19:07 PDT
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?
Comment 4 Gyuyoung Kim 2013-03-15 00:36:06 PDT
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 ?
Comment 5 Jinwoo Song 2013-03-15 03:04:49 PDT
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.
Comment 6 Jinwoo Song 2013-03-15 03:12:13 PDT
Created attachment 193270 [details]
Patch

Applied the comments by Ryuan and Gyuyoung.
Comment 7 Gyuyoung Kim 2013-03-17 00:10:22 PDT
Comment on attachment 193270 [details]
Patch

LGTM now.
Comment 8 WebKit Review Bot 2013-03-17 03:46:27 PDT
Comment on attachment 193270 [details]
Patch

Clearing flags on attachment: 193270

Committed r146007: <http://trac.webkit.org/changeset/146007>
Comment 9 WebKit Review Bot 2013-03-17 03:46:32 PDT
All reviewed patches have been landed.  Closing bug.