[webkit-reviews] review granted: [Bug 108997] More updates to Caption user preferences : [Attachment 186743] Proposed patch

[bugzilla-daemon at webkit.org]

Dean Jackson <dino at apple.com> has granted Eric Carlson
<eric.carlson at apple.com>'s request for review:
Bug 108997: More updates to Caption user preferences
https://bugs.webkit.org/show_bug.cgi?id=108997

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

------- Additional Comments from Dean Jackson <dino at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=186743&action=review


> Source/WebCore/ChangeLog:8
> +	   No new tests (OOPS!).

Remove this line. I guess technically we could test some of these if we toggled
captions on, scrubbed the media and then were able to dump the shadow root for
the captions.

> Source/WebCore/page/CaptionUserPreferencesMac.mm:151
> +    builder.append(": .2em");

Does this value come from the spec? It might be better as a defined constant
somewhere.

> Source/WebCore/page/CaptionUserPreferencesMac.mm:224
>      builder.append(':');
> -    builder.append(String::format("%.02f", radius));
> +    builder.append(String::format("%.02fpx", radius));

Maybe combine the ":" into String::format?