Bug 58866 – [EFL] Add current time to media control panel

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

[Gyuyoung Kim]

I implement paintMediaCurrentTime() to show play time of media content. I refer to gtk implementation.

[Gyuyoung Kim]

Created attachment 90153 [details]
Proposed Patch

[Gyuyoung Kim]

Created attachment 90156 [details]
Screen Capture - Timeline

[Gyuyoung Kim]

Created attachment 90177 [details]
Modified Patch

Fix return value in paintMediaFullscreenButton()

[Gyuyoung Kim]

(In reply to comment #3)
> Created an attachment (id=90177) [details]
> Modified Patch
> 
> Fix return value in paintMediaFullscreenButton()

(In reply to comment #3)
> Created an attachment (id=90177) [details]
> Modified Patch
> 
> Fix return value in paintMediaFullscreenButton()

Oops, I fix paintMediaCurrentTime(), not paintMediaFullscreenButton().

[Lucas De Marchi]

Comment on attachment 90177 [details]
Modified Patch

LGTM

[Daniel Bates]

Comment on attachment 90177 [details]
Modified Patch

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

r=me

> Source/WebCore/platform/efl/RenderThemeEfl.cpp:694
> +    , m_panelColor(220, 220, 195)

Nit: It might be nice to write an inline comment that explains that the RGB values describe some kind of beige/tan-ish color.

> Source/WebCore/platform/efl/RenderThemeEfl.cpp:1101
> +    return formatMediaControlsTime(currentTime) + " / " + formatMediaControlsTime(duration);

It is more efficient to use makeString() (defined in wtf/text/StringConcatenate.h) for such string concatenation. Notice, there is a three argument variant.

> Source/WebCore/platform/efl/RenderThemeEfl.cpp:1196
> +

This variable is being underutilized since it's only referenced once in this function body. I suggest inlining the value of this variable into the line below and removing this variable.

[Gyuyoung Kim]

Created attachment 90310 [details]
Modified Patch

Daniel, thank you for your review.  

I modify this patch according to your comment. It seems no nits. So, I land this patch.

[Gyuyoung Kim]

Created attachment 90311 [details]
Modified Patch - Add reviewer field

[WebKit Commit Bot]

Comment on attachment 90311 [details]
Modified Patch - Add reviewer field

Clearing flags on attachment: 90311

Committed r84348: <http://trac.webkit.org/changeset/84348>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.