[Webkit-unassigned] [Bug 117008] [WK2][GTK] add support for subtitles on webkit2GTK

[bugzilla-daemon at webkit.org]

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


Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #209033|review?                     |review+
               Flag|                            |




--- Comment #20 from Martin Robinson <mrobinson at webkit.org>  2013-08-18 07:59:31 PST ---
(From update of attachment 209033)
View in context: https://bugs.webkit.org/attachment.cgi?id=209033&action=review

Looks good, but please consider fixing a few nits before landing...

> Source/WebCore/html/shadow/MediaControlsGtk.cpp:270
> +    if (m_closedCaptionsContainer) {

Maybe an early return here instead?

> Source/WebCore/html/shadow/MediaControlsGtk.cpp:272
> +        if (m_closedCaptionsContainer->isShowing())
> +            hideClosedCaptionTrackList();

Ditto.

> Source/WebCore/html/shadow/MediaControlsGtk.cpp:291
> +    m_closedCaptionsContainer->show();
> +
> +    m_panel->setInlineStyleProperty(CSSPropertyPointerEvents, CSSValueNone);

You can probably remove this empty line.

> Source/WebCore/html/shadow/MediaControlsGtk.cpp:301
> +    m_closedCaptionsContainer->hide();
> +
> +    m_panel->removeInlineStyleProperty(CSSPropertyPointerEvents);

Ditto.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:147
> +    prefs->setShouldDisplaySubtitles(TRUE);
>      ExperimentalFeatures features;

On the other hand, this probably deserves an empty newline. :)

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1136
>              readWriteConstructParamFlags));
> -
>  }

I think this change is left over.

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