Bug 39269

Summary:

[Chromium] Windows: Determinate progress bar should have glossy overlay

Product:

WebKit

Reporter:

Hajime Morrita <morrita>

Component:

Platform

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

tkent

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

OS X 10.5

Attachments:

Description	Flags
patch v0	none
patch v1	none
patch v2	none
patch v3; update ChangeLog	tkent: review+

Hajime Morrita

Reported 2010-05-17 23:14:22 PDT

On Windows Vista with Aero, determinate progress bars are drawn with glossy overlay. We should have it for <progress> on Windows Chromium.

Attachments
patch v0 (7.66 KB, patch) 2010-05-17 23:37 PDT, Hajime Morrita	no flags	Details Formatted Diff Diff
patch v1 (8.36 KB, patch) 2010-05-18 03:04 PDT, Hajime Morrita	no flags	Details Formatted Diff Diff
patch v2 (8.37 KB, patch) 2010-05-18 03:10 PDT, Hajime Morrita	no flags	Details Formatted Diff Diff
patch v3; update ChangeLog (8.38 KB, patch) 2010-05-18 04:27 PDT, Hajime Morrita	tkent: review+	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Hajime Morrita

Comment 1 2010-05-17 23:19:52 PDT

Chromium side is http://crbug.com/44430

Hajime Morrita

Comment 2 2010-05-17 23:37:10 PDT

Created attachment 56327 [details] patch v0

Kent Tamura

Comment 3 2010-05-18 01:24:29 PDT

Comment on attachment 56327 [details] patch v0 WebCore/platform/chromium/ChromiumBridge.h:228 + GraphicsContext*, const IntRect& barRect, const IntRect& valueRect, bool determinate, double time); The name "time" is unclear. Maybe "secondsSinceEpoch"? WebCore/rendering/RenderThemeChromiumWin.cpp:672 + // So here we can return arbitrary value. The value must be greater than 0, not arbitrary. WebKit/chromium/public/WebThemeEngine.h:81 + bool determinate, double time) {} Ditto for "time" WebKit/chromium/src/ChromiumBridge.cpp:683 + GraphicsContext* gc, const IntRect& barRect, const IntRect& valueRect, bool determinate, double time) ditto. WebKit/chromium/src/ChromiumBridge.cpp:686 + gc->platformContext()->canvas(), barRect, valueRect, determinate, time); ditto.

Hajime Morrita

Comment 4 2010-05-18 03:04:09 PDT

Created attachment 56344 [details] patch v1

Hajime Morrita

Comment 5 2010-05-18 03:10:41 PDT

Created attachment 56347 [details] patch v2

Hajime Morrita

Comment 6 2010-05-18 03:11:10 PDT

Hi Kent-san, thank you for reviewing quickly! I updated the patch to take the feedback. > WebCore/platform/chromium/ChromiumBridge.h:228 > + GraphicsContext*, const IntRect& barRect, const IntRect& valueRect, bool determinate, double time); > The name "time" is unclear. Maybe "secondsSinceEpoch"? Fixed to replace with "animatedSeconds". I fixed other part too. This patch also chagnes the value from absolute seconds (WTF::currentTime()) to relative seconds from animation started to make the animation starts from left side.

Kent Tamura

Comment 7 2010-05-18 03:19:52 PDT

Comment on attachment 56347 [details] patch v2 Please update ChangeLog. It has no entry for RenderProgress.h.

Hajime Morrita

Comment 8 2010-05-18 04:27:01 PDT

Created attachment 56359 [details] patch v3; update ChangeLog

Hajime Morrita

Comment 9 2010-05-18 04:28:01 PDT

> Please update ChangeLog. > It has no entry for RenderProgress.h. Oops. I'm sorry for that. I've updated the patch to fix it.

Kent Tamura

Comment 10 2010-05-19 21:33:13 PDT

Can we close this?

Hajime Morrita

Comment 11 2010-05-20 21:00:22 PDT

Committed r59733: <http://trac.webkit.org/changeset/59733> Closing bug.

Note You need to log in before you can comment on or make changes to this bug.