Bug 53906

Summary:

[EFL] Add dummy functions for HTML5 Video's control UI

Product:

WebKit

Reporter:

Gyuyoung Kim <gyuyoung.kim>

Component:

WebKit EFL

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, eric, kenneth, leandro, lucas.de.marchi, tonikitoo

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

Linux

Attachments:

Description	Flags
Patch	none

Gyuyoung Kim

Reported 2011-02-06 23:45:11 PST

In WebKit EFL, there are no implementations for HTML5 control UI. I am implementing the functions. First of all, I'd like to add dummy functions for them.

Attachments
Patch (5.50 KB, patch) 2011-02-06 23:48 PST, Gyuyoung Kim	no flags	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Gyuyoung Kim

Comment 1 2011-02-06 23:48:29 PST

Created attachment 81453 [details] Patch

Eric Seidel (no email)

Comment 2 2011-02-07 15:22:19 PST

Comment on attachment 81453 [details] Patch OK. So currently the compile fails if you enable VIDEO w/o these? Or does it just use the defaults? if it uses the default implementations then it seems these are implemented incorrectly (and should instead call the parent function.

Gyuyoung Kim

Comment 3 2011-02-07 15:50:49 PST

(In reply to comment #2) > (From update of attachment 81453 [details]) > OK. So currently the compile fails if you enable VIDEO w/o these? Or does it just use the defaults? if it uses the default implementations then it seems these are implemented incorrectly (and should instead call the parent function. In WebKit EFL, VIDEO feature is enabled by default. So, these functions will be used by default. Currently, these functions which paint players's control UI are defined in RenderTheme.h file virtually. 283 virtual bool paintMediaFullscreenButton(RenderObject*, const PaintInfo&, const IntRect&) { return true; } 284 virtual bool paintMediaPlayButton(RenderObject*, const PaintInfo&, const IntRect&) { return true; } 285 virtual bool paintMediaMuteButton(RenderObject*, const PaintInfo&, const IntRect&) { return true; } 286 virtual bool paintMediaSeekBackButton(RenderObject*, const PaintInfo&, const IntRect&) { return true; } 287 virtual bool paintMediaSeekForwardButton(RenderObject*, const PaintInfo&, const IntRect&) { return true; } 288 virtual bool paintMediaSliderTrack(RenderObject*, const PaintInfo&, const IntRect&) { return true; } 289 virtual bool paintMediaSliderThumb(RenderObject*, const PaintInfo&, const IntRect&) { return true; } 290 virtual bool paintMediaVolumeSliderContainer(RenderObject*, const PaintInfo&, const IntRect&) { return true; } 291 virtual bool paintMediaVolumeSliderTrack(RenderObject*, const ... Now, the parent functions just return "true" because WebKit EFL doesn't implement these functions yet. So, first of all, I add dummy functions, then I'd like to implement these functions step by step. If I have misunderstanding, please let me know. Thank you. :-)

Eric Seidel (no email)

Comment 4 2011-02-07 21:57:47 PST

Comment on attachment 81453 [details] Patch And remind me what the returning false vs. true means? (Yes, we really really really need to change that to be an enum).

Gyuyoung Kim

Comment 5 2011-02-08 17:29:13 PST

(In reply to comment #4) > (From update of attachment 81453 [details]) > And remind me what the returning false vs. true means? (Yes, we really really really need to change that to be an enum). paintMediaXXXX functions are invoked by below function. bool RenderTheme::paint(RenderObject* o, const PaintInfo& paintInfo, const IntRect& r) AFAIK, this paint function draws media control UI. And, the paint() is called by RenderBox::paintBoxDecorationsWithSize() as below, void RenderBox::paintBoxDecorationsWithSize(PaintInfo& paintInfo, int tx, int ty, int width, int height) { ... // If we have a native theme appearance, paint that before painting our background. // The theme will tell us whether or not we should also paint the CSS background. bool themePainted = style()->hasAppearance() && !theme()->paint(this, paintInfo, IntRect(tx, ty, width, height)); => This line calls the paint function. if (!themePainted) { // The <body> only paints its background if the root element has defined a background // independent of the body. Go through the DOM to get to the root element's render object, // since the root could be inline and wrapped in an anonymous block. if (!isBody() || document()->documentElement()->renderer()->hasBackground()) paintFillLayers(paintInfo, style()->visitedDependentColor(CSSPropertyBackgroundColor), style()->backgroundLayers(), tx, ty, width, height); if (style()->hasAppearance()) theme()->paintDecorations(this, paintInfo, IntRect(tx, ty, width, height)); } It seems to me that the "true" or "false" return value mean whether each port paints UI theme or not. Thus, I think this is correct that EFL port returns "false". Because, it doesn't paint nothing until implementing these dummy functions. If we need to change the boolean return value with enum type, we should modify RenderBox::paintBoxDecorationWithSize and each port's paint functions together. Do you think we should modify now ?

Eric Seidel (no email)

Comment 6 2011-02-09 02:00:15 PST

(In reply to comment #5) > (In reply to comment #4) > > (From update of attachment 81453 [details] [details]) > If we need to change the boolean return value with enum type, we should modify RenderBox::paintBoxDecorationWithSize and each port's paint functions together. Do you think we should modify now ? Nah. That's a separate patch.

Eric Seidel (no email)

Comment 7 2011-02-09 02:01:32 PST

Comment on attachment 81453 [details] Patch OK.

WebKit Commit Bot

Comment 8 2011-02-09 03:54:34 PST

Comment on attachment 81453 [details] Patch Clearing flags on attachment: 81453 Committed r78041: <http://trac.webkit.org/changeset/78041>

WebKit Commit Bot

Comment 9 2011-02-09 03:54:39 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.