[Webkit-unassigned] [Bug 28689] Replace disabled media mute button with port-specific implementation.

[bugzilla-daemon at webkit.org]

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


Eric Carlson <eric.carlson at apple.com> changed:

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




--- Comment #10 from Eric Carlson <eric.carlson at apple.com>  2009-09-22 08:51:09 PDT ---
(From update of attachment 39900)

> +        Sample implementation for port-specific rendering of media controls.

"Sample implementation", what do you mean?


> +    // Alternatively we could pass in the theme for fine-tuned adjustment.  If
> +    // we keep it as simply setting display to NONE then I might rename this method
> +    // shouldDisplayMediaControlPart instead.
> +    if (!renderer->theme()->shouldRenderMediaControlPart(renderer))
> +        style->setDisplay(NONE);

This shouldn't be necessary. I think if have both MediaControlElement::
rendererIsNeeded and MediaControlInputElement:: rendererIsNeeded call the theme
method it should just work.


>  bool MediaControlMuteButtonElement::rendererIsNeeded(RenderStyle* style)
>  {
> -    return MediaControlInputElement::rendererIsNeeded(style) && !disabled();
> +    // All subclasses will probably end up NOT implementing this method anymore.
> +    return MediaControlInputElement::rendererIsNeeded(style);

Why not just remove the method completely?


> +bool RenderTheme::shouldRenderMediaControlPart(RenderObject* o)
> +{
> +    HTMLMediaElement* mediaElement = static_cast<MediaControlInputElement*>(o->node())->mediaElement();
> +    switch (o->style()->appearance()) {
> +    case MediaMuteButtonPart:
> +        return mediaElement->hasAudio();
> +    // More media parts can go here, but then we end up moving localized logic
> +    // from the MediaControl elements into a big switch statement here, which
> +    // kind of sucks.

"which kind of sucks" is probably not the most helpful comment to someone
coming along later.

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