[Webkit-unassigned] [Bug 135677] Increase width of caption container if a larger font size is selected from user prefs

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Aug 7 10:52:48 PDT 2014


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





--- Comment #8 from Brent Fulgham <bfulgham at webkit.org>  2014-08-07 10:52:57 PST ---
(From update of attachment 236195)
View in context: https://bugs.webkit.org/attachment.cgi?id=236195&action=review

> Source/WebCore/ChangeLog:14
> +        

Extra space isn't needed.

> Source/WebCore/html/track/TextTrackCueGeneric.cpp:80
> +        float authorFontSize = fmax(defaultFontSize, videoSize.height() * cue->baseFontSizeRelativeToVideoHeight() / 100);

We should be using std::max or maybe std::max<float> here.
defaultFontSize should be VTTCueBox::DEFAULT_FONT_SIZE.

> Source/WebCore/html/track/TextTrackCueGeneric.cpp:84
> +        float multiplier = fmax(1.0, m_fontSizeFromCaptionUserPrefs / authorFontSize);

Ditto.

> Source/WebCore/html/track/TextTrackCueGeneric.cpp:86
> +            setInlineStyleProperty(CSSPropertyWidth, size*multiplier, CSSPrimitiveValue::CSS_PERCENTAGE);

Missing spaces around the '*'.

> Source/WebCore/html/track/TextTrackCueGeneric.cpp:88
> +            setInlineStyleProperty(CSSPropertyHeight, size*multiplier,  CSSPrimitiveValue::CSS_PERCENTAGE);

Ditto.

> Source/WebCore/html/track/VTTCue.cpp:173
> +    float multiplier = fmax(1.0, m_fontSizeFromCaptionUserPrefs / defaultFontSize);

std::max or std::max<float>, please. VTTCueBox::DEFAULT_FONT_SIZE

> Source/WebCore/html/track/VTTCue.cpp:176
> +        setInlineStyleProperty(CSSPropertyWidth, static_cast<double>(m_cue.getCSSSize()*multiplier), CSSPrimitiveValue::CSS_PERCENTAGE);

Need spaces around the '*'.

> Source/WebCore/html/track/VTTCue.cpp:181
> +        setInlineStyleProperty(CSSPropertyHeight, static_cast<double>(m_cue.getCSSSize()*multiplier),  CSSPrimitiveValue::CSS_PERCENTAGE);

Ditto.

> Source/WebCore/html/track/VTTCue.h:72
> +    static const float defaultFontSize;

I think this should be named DEFAULT_FONT_SIZE

> Source/WebCore/html/track/VTTCue.h:74
>  

We should add a comment along the lines of:
           // This default value must be the same as the one specified in mediaControlsApple.css for -webkit-media-controls-closed-captions-container

> Source/WebCore/html/track/VTTCue.h:75
> +const float VTTCueBox::defaultFontSize = 10;

... then this becomes VTTCueBox::DEFAULT_FONT_SIZE

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



More information about the webkit-unassigned mailing list