[webkit-reviews] review granted: [Bug 114230] [Mac] user caption styles not applied to correct element : [Attachment 196991] Updated patch.

[bugzilla-daemon at webkit.org]

Jer Noble <jer.noble at apple.com> has granted Eric Carlson
<eric.carlson at apple.com>'s request for review:
Bug 114230: [Mac] user caption styles not applied to correct element
https://bugs.webkit.org/show_bug.cgi?id=114230

Attachment 196991: Updated patch.
https://bugs.webkit.org/attachment.cgi?id=196991&action=review

------- Additional Comments from Jer Noble <jer.noble at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=196991&action=review


> Source/WebCore/html/shadow/MediaControlElements.cpp:1326
> +    m_fontSize = lrintf(smallestDimension *
(document()->page()->group().captionPreferences()->captionFontSizeScale(m_fontS
izeIsImportant)));

This is kind of confusing, that m_fontSizeIsImportant is modified by
captionFontSizeScale().  Perhaps captionFontSizeScaleAndImportance(), and
breaking up this into two lines?:

float fontScale =
document()->page()->group().captionPreferences()->captionFontSizeScale(m_fontSi
zeIsImportant);
m_fontSize = lrintf(smallestDimension * fontScale);

> Source/WebCore/html/shadow/MediaControls.cpp:415
>  void MediaControls::textTrackPreferencesChanged()
>  {
> +    closedCaptionTracksChanged();
>      if (m_textDisplayContainer)
>	   m_textDisplayContainer->updateSizes(true);
> -    closedCaptionTracksChanged();
>  }

Could this be pulled into another bug? It seems unrelated.