Bug 108997 - More updates to Caption user preferences
Summary: More updates to Caption user preferences
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Carlson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-02-05 18:38 PST by Eric Carlson
Modified: 2013-02-05 21:15 PST (History)
6 users (show)

See Also:


Attachments
Proposed patch (12.87 KB, patch)
2013-02-05 18:48 PST, Eric Carlson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Carlson 2013-02-05 18:38:15 PST
Refine the code the Mac port uses to modify caption and subtitle style.
Comment 1 Radar WebKit Bug Importer 2013-02-05 18:38:37 PST
<rdar://problem/13159553>
Comment 2 Eric Carlson 2013-02-05 18:48:50 PST
Created attachment 186743 [details]
Proposed patch
Comment 3 Dean Jackson 2013-02-05 20:28:58 PST
Comment on attachment 186743 [details]
Proposed patch

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?
Comment 4 Build Bot 2013-02-05 20:32:21 PST
Comment on attachment 186743 [details]
Proposed patch

Attachment 186743 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/16388093
Comment 5 Eric Carlson 2013-02-05 21:01:27 PST
(In reply to comment #4)
> (From update of attachment 186743 [details])
> Attachment 186743 [details] did not pass win-ews (win):
> Output: http://queues.webkit.org/results/16388093

I don't think so:

1>####### COMPILING 1 FILES USING AT MOST 8 PARALLEL INSTANCES OF cl.exe ###########
1>ImageDiffCG.cpp
1>..\cg\ImageDiffCG.cpp(37) : fatal error C1083: Cannot open include file: 'wtf/Platform.h': No such file or directory
Comment 6 Eric Carlson 2013-02-05 21:15:04 PST
Committed r141966: <http://trac.webkit.org/changeset/141966>