Bug 131258 - Honor System-Level User Preferences for Captions Display
Summary: Honor System-Level User Preferences for Captions Display
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords: InRadar
Depends on:
Blocks: 131344
  Show dependency treegraph
 
Reported: 2014-04-04 21:15 PDT by Brent Fulgham
Modified: 2014-04-07 20:52 PDT (History)
11 users (show)

See Also:


Attachments
Patch (3.06 KB, patch)
2014-04-04 21:24 PDT, Brent Fulgham
eric.carlson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2014-04-04 21:15:06 PDT
WebKit doesn't properly honor the global user preferences for caption display in a few cases:

1. When a video starts playing, it should show captions if the user's preferences indicate a preference for caption display.
2. While a video is playing, captions should turn on/off as the user changes preferences.
3. Regardless of the user's preferences, if the user manually selects captions they should be displayed.

In all cases, the result of the following JavaScript should reflect what's going on in the video playback:

document.querySelector("video").webkitClosedCaptionsVisible
Comment 1 Brent Fulgham 2014-04-04 21:15:55 PDT
<rdar://problem/15745400>
<rdar://problem/15745452>
Comment 2 Brent Fulgham 2014-04-04 21:24:30 PDT
Created attachment 228653 [details]
Patch
Comment 3 Brent Fulgham 2014-04-04 21:27:35 PDT
Comment on attachment 228653 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=228653&action=review

> Source/WebCore/html/HTMLMediaElement.cpp:-3713
> -        m_captionDisplayMode = displayMode;

I needed to do this so that the "ForceOn" state was properly seen by system.
Comment 4 Brent Fulgham 2014-04-04 22:20:17 PDT
All tests (including uncommitted region tests) pass with this change.
Comment 5 Jon Lee 2014-04-05 14:22:45 PDT
(In reply to comment #4)
> All tests (including uncommitted region tests) pass with this change.

Additional test case, then?
Comment 6 Brent Fulgham 2014-04-05 20:04:36 PDT
(In reply to comment #5)
> (In reply to comment #4)
> > All tests (including uncommitted region tests) pass with this change.
> 
> Additional test case, then?

after looking at it, I  realize it's just a local copy of one of the Opera tests. I'm trying to get the whole set of them turned on in an upcoming patch.
Comment 7 Brent Fulgham 2014-04-06 16:22:15 PDT
Committed r166858: <http://trac.webkit.org/changeset/166858>