Bug 89140

Summary:

[EFL][WK2] Add ewk_view_cursor_set to change cursor.

Product:

WebKit

Reporter:

Ryuan Choi <ryuan.choi>

Component:

WebKit EFL

Assignee:

Ryuan Choi <ryuan.choi>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

gyuyoung.kim, gyuyoung.kim, lucas.de.marchi, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

90107

Bug Blocks:

61838

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
rebased	none

Ryuan Choi

Reported 2012-06-14 17:11:53 PDT

WebKit2/Efl can receive setCursor from WebProcess after r120369, but implementation is missing.

Attachments
Patch (5.08 KB, patch) 2012-06-14 17:20 PDT, Ryuan Choi	no flags	Details Formatted Diff Diff
Patch (7.16 KB, patch) 2012-07-17 01:42 PDT, Ryuan Choi	no flags	Details Formatted Diff Diff
Patch (7.31 KB, patch) 2012-07-18 01:22 PDT, Ryuan Choi	no flags	Details Formatted Diff Diff
rebased (7.33 KB, patch) 2012-07-23 16:49 PDT, Ryuan Choi	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Ryuan Choi

Comment 1 2012-06-14 17:20:11 PDT

Created attachment 147684 [details] Patch

Chris Dumez

Comment 2 2012-06-25 04:47:38 PDT

Comment on attachment 147684 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=147684&action=review > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:553 > + WebCore::applyFallbackCursor(ecoreEvas, group); I'm not familiar with this code but why do we have only a fallback implementation here? It looks odd. If I look at the WK1 implementation, we have a normal implementation AND a fallback.

Ryuan Choi

Comment 3 2012-06-27 16:16:29 PDT

(In reply to comment #2) > (From update of attachment 147684 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=147684&action=review > > > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:553 > > + WebCore::applyFallbackCursor(ecoreEvas, group); > > I'm not familiar with this code but why do we have only a fallback implementation here? It looks odd. If I look at the WK1 implementation, we have a normal implementation AND a fallback. OK, if then I will revise this after Bug 900107 is landed because implementation of WebKit1/Efl needs theme.

Ryuan Choi

Comment 4 2012-07-17 01:42:23 PDT

Created attachment 152718 [details] Patch

Chris Dumez

Comment 5 2012-07-17 23:24:54 PDT

Comment on attachment 152718 [details] Patch LGTM.

Gyuyoung Kim

Comment 6 2012-07-17 23:39:46 PDT

Comment on attachment 152718 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=152718&action=review > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:818 > + width = 16; I think it is better to use constant variable for width and height.

Ryuan Choi

Comment 7 2012-07-18 01:22:11 PDT

Created attachment 152961 [details] Patch

Ryuan Choi

Comment 8 2012-07-18 01:25:39 PDT

(In reply to comment #6) > (From update of attachment 152718 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=152718&action=review > > > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:818 > > + width = 16; > > I think it is better to use constant variable for width and height. Ok, I add defaultCursorSize for it.

Gyuyoung Kim

Comment 9 2012-07-18 06:41:24 PDT

Comment on attachment 152961 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=152961&action=review Looks good > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:334 > + g_parentSmartClass.add(ewkView); I wonder why do you move this line ? Do we need to add a view to parent smart class though view object can't get _Ewk_View_Private_Data ? > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:336 > + smartData->priv = _ewk_view_priv_new(smartData); Should we move this line ?

Ryuan Choi

Comment 10 2012-07-18 07:31:46 PDT

(In reply to comment #9) > (From update of attachment 152961 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=152961&action=review > > Looks good > > > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:334 > > + g_parentSmartClass.add(ewkView); > > I wonder why do you move this line ? Do we need to add a view to parent smart class though view object can't get _Ewk_View_Private_Data ? Thank you. By calling `add` of parent smart class, child instance like ewkView initialize internal objects of smart data such as base.evas. > > > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:336 > > + smartData->priv = _ewk_view_priv_new(smartData); > > Should we move this line ? Yes. Before calling `add` of parent smart class, smartData is not concrete to use. so previous position is wrong. And because this patch need to use smartData in _ewk_view_priv_new, we should move it to right position.

Gyuyoung Kim

Comment 11 2012-07-18 07:38:28 PDT

Comment on attachment 152961 [details] Patch LGTM.

Ryuan Choi

Comment 12 2012-07-23 16:49:52 PDT

Created attachment 153899 [details] rebased

WebKit Review Bot

Comment 13 2012-07-25 02:16:35 PDT

Comment on attachment 153899 [details] rebased Clearing flags on attachment: 153899 Committed r123593: <http://trac.webkit.org/changeset/123593>

WebKit Review Bot

Comment 14 2012-07-25 02:16:40 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.