[Webkit-unassigned] [Bug 26304] [GTK] Add controls for playing html5 video.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=26304


Xan Lopez <xan.lopez at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #40178|review?                     |review-
               Flag|                            |




--- Comment #20 from Xan Lopez <xan.lopez at gmail.com>  2009-10-05 01:39:04 PDT ---
(From update of attachment 40178)
>@@ -3433,6 +3434,7 @@ webcore_dist += \
> 	WebCore/bindings/scripts/InFilesParser.pm \
> 	WebCore/bindings/scripts/generate-bindings.pl \
> 	WebCore/bindings/scripts/CodeGeneratorJS.pm \
>+	WebCore/css/gtk/mediaControlsGtk.css
> 	WebCore/css/html.css \
> 	WebCore/css/quirks.css \
> 	WebCore/css/view-source.css \

Missing \ at the end of the new line, this won't work.

Also:

../../WebCore/platform/gtk/RenderThemeGtk.cpp: In member function ‘virtual
WebCore::String WebCore::RenderThemeGtk::extraMediaControlsStyleSheet()’:
../../WebCore/platform/gtk/RenderThemeGtk.cpp:598: error:
‘mediaControlsGtkUserAgentStyleSheet’ was not declared in this scope
make[1]: *** [WebCore/platform/gtk/libWebCore_la-RenderThemeGtk.lo] Error 1
make[1]: Leaving directory `/home/xan/git/WebKit/build/normal'

Leftover from other patches?

Other than that, a couple of comments (after fixing the patch to compile):

>+static Color panelColor = 0;
>+static Color sliderColor = 0;
>+static Color sliderThumbColor = 0;
>+
>+// Names of these icons can vary because of text direction.
>+static char* playButtonIconName = 0;
>+static char* seekBackButtonIconName = 0;
>+static char* seekForwardButtonIconName = 0;
>+
>+static int mediaIconSize = 16;
>+static int mediaSliderHeight = 14;

This should be const. Also, I'd say these sizes should be calculated
dynamically, maybe based on the size of the video element we are showing?

>+#if ENABLE(VIDEO)
>+    initMediaStyling(gtk_rc_get_style(GTK_WIDGET(gtkContainer())));
>+#endif

Do you actually need to use gtk_rc_get_style or would gtk_widget_get_style be
enough?

The progress bar does not seem to work at all in my testing? (Eg,
http://people.mozilla.com/~vladimir/mdemos/video1.html).

The controls seem to be over the video content, is this the intended behavior?
The spec does not seem to say much about it though.

Marking as r- for now.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.