Bug 125034

Summary:

[EFL] Implement scrollbarThickness for opaque scrollbar

Product:

WebKit

Reporter:

Ryuan Choi <ryuan.choi>

Component:

WebKit EFL

Assignee:

Ryuan Choi <ryuan.choi>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cdumez, commit-queue, gyuyoung.kim, lucas.de.marchi, rakuco

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none

Ryuan Choi

Reported 2013-11-30 18:02:08 PST

WebKitEfl draws transparent scrollbar on the top of contents using edje. This patch give a chance to support opaque scrollbar by edje.

Attachments
Patch (4.16 KB, patch) 2013-11-30 18:12 PST, Ryuan Choi	no flags	Details Formatted Diff Diff
Patch (4.31 KB, patch) 2013-11-30 18:44 PST, Ryuan Choi	no flags	Details Formatted Diff Diff
Patch (4.51 KB, patch) 2013-11-30 21:55 PST, Ryuan Choi	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Ryuan Choi

Comment 1 2013-11-30 18:12:13 PST

Created attachment 218095 [details] Patch

Ryuan Choi

Comment 2 2013-11-30 18:44:34 PST

Created attachment 218097 [details] Patch

Gyuyoung Kim

Comment 3 2013-11-30 21:28:04 PST

Comment on attachment 218097 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=218097&action=review It looks this patch need to be verified if there is any layout test regression. > Source/WebCore/platform/efl/RenderThemeEfl.cpp:518 > + ScrollbarThemeEfl* scrollbarTheme = static_cast<ScrollbarThemeEfl*>(ScrollbarTheme::theme()); Don't we need to use local variable as below ? static_cast<ScrollbarThemeEfl*>(ScrollbarTheme::theme())->setScrollbarThickness(atoi(thickness)); > Source/WebCore/platform/efl/ScrollbarThemeEfl.h:39 > + virtual int scrollbarThickness(ScrollbarControlSize = RegularScrollbar) { return m_scrollbarThickness; } Missing OVERRIDE FINAL keyword. > Source/WebCore/platform/efl/ScrollbarThemeEfl.h:41 > + virtual void registerScrollbar(ScrollbarThemeClient*) { } ditto. > Source/WebCore/platform/efl/ScrollbarThemeEfl.h:42 > + virtual void unregisterScrollbar(ScrollbarThemeClient*) { } ditto.

Ryuan Choi

Comment 4 2013-11-30 21:55:55 PST

Created attachment 218103 [details] Patch

Ryuan Choi

Comment 5 2013-11-30 21:57:20 PST

(In reply to comment #3) > (From update of attachment 218097 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=218097&action=review > > It looks this patch need to be verified if there is any layout test regression. > Because this does not change with default theme, it must be fine. > > Source/WebCore/platform/efl/RenderThemeEfl.cpp:518 > > + ScrollbarThemeEfl* scrollbarTheme = static_cast<ScrollbarThemeEfl*>(ScrollbarTheme::theme()); > > Don't we need to use local variable as below ? > static_cast<ScrollbarThemeEfl*>(ScrollbarTheme::theme())->setScrollbarThickness(atoi(thickness)); > Sure, fixed > > Source/WebCore/platform/efl/ScrollbarThemeEfl.h:39 > > + virtual int scrollbarThickness(ScrollbarControlSize = RegularScrollbar) { return m_scrollbarThickness; } > > Missing OVERRIDE FINAL keyword. > > > Source/WebCore/platform/efl/ScrollbarThemeEfl.h:41 > > + virtual void registerScrollbar(ScrollbarThemeClient*) { } > > ditto. > > > Source/WebCore/platform/efl/ScrollbarThemeEfl.h:42 > > + virtual void unregisterScrollbar(ScrollbarThemeClient*) { } > > ditto. Sure, I did.

Gyuyoung Kim

Comment 6 2013-11-30 22:03:04 PST

Comment on attachment 218103 [details] Patch LGTM.

WebKit Commit Bot

Comment 7 2013-11-30 23:03:03 PST

Comment on attachment 218103 [details] Patch Clearing flags on attachment: 218103 Committed r159897: <http://trac.webkit.org/changeset/159897>

WebKit Commit Bot

Comment 8 2013-11-30 23:03:06 PST

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.