Summary: | [Mac] .webkitClosedCaptionsVisible doesn't work with "Automatic" caption mode | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Carlson <eric.carlson> | ||||||||
Component: | Media | Assignee: | Eric Carlson <eric.carlson> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | ahmad.saleem792, commit-queue, dfarler, dino, esprehn+autocc, glenn, jer.noble | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | 118333 | ||||||||||
Bug Blocks: | |||||||||||
Attachments: |
|
Description
Eric Carlson
2013-07-02 12:40:39 PDT
Created attachment 205938 [details]
Proposed patch
Committed in r152318: https://trac.webkit.org/r152318 Re-opened since this is blocked by bug 118333 *** Bug 118332 has been marked as a duplicate of this bug. *** Created attachment 206044 [details]
Patch that causes fewer regressions!
Created attachment 206045 [details]
Patch that causes fewer regressions, AND with a complete ChangeLog!
Comment on attachment 206045 [details] Patch that causes fewer regressions, AND with a complete ChangeLog! View in context: https://bugs.webkit.org/attachment.cgi?id=206045&action=review > Source/WebCore/html/HTMLMediaElement.cpp:301 > , m_closedCaptionsVisible(false) > + , m_webkitClosedCaptionsVisible(false) It seems awkward to have these two ivars named so similarly. How about m_webkitLegacyClosedCaptionOverride? > Source/WebCore/html/HTMLMediaElement.cpp:4530 > + m_webkitClosedCaptionsVisible = visible; This would be: m_webkitLegacyClosedCaptionOverride = visible; > Source/WebCore/html/HTMLMediaElement.cpp:4532 > + m_webkitClosedCaptionsVisible = m_closedCaptionsVisible; You can remove this line if... (ctd) > Source/WebCore/html/HTMLMediaElement.cpp:4537 > - return m_closedCaptionsVisible; > + return m_webkitClosedCaptionsVisible; You rename this: m_closedCaptionsVisible && m_webkitLegacyClosedCaptionOverride; (In reply to comment #9) > (From update of attachment 206045 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=206045&action=review > > > Source/WebCore/html/HTMLMediaElement.cpp:301 > > , m_closedCaptionsVisible(false) > > + , m_webkitClosedCaptionsVisible(false) > > It seems awkward to have these two ivars named so similarly. How about m_webkitLegacyClosedCaptionOverride? > > > Source/WebCore/html/HTMLMediaElement.cpp:4530 > > + m_webkitClosedCaptionsVisible = visible; > > This would be: m_webkitLegacyClosedCaptionOverride = visible; > > > Source/WebCore/html/HTMLMediaElement.cpp:4532 > > + m_webkitClosedCaptionsVisible = m_closedCaptionsVisible; > > You can remove this line if... (ctd) > > > Source/WebCore/html/HTMLMediaElement.cpp:4537 > > - return m_closedCaptionsVisible; > > + return m_webkitClosedCaptionsVisible; > > You rename this: m_closedCaptionsVisible && m_webkitLegacyClosedCaptionOverride; All good suggestions, I will make them before landing. Thanks! Committed in r152422: https://trac.webkit.org/r152422 Landed finally with this but didn't get marked as 'RESOLVED FIXED': https://github.com/WebKit/WebKit/commit/97a3125e7fc58bc3583d6ca841d59eb507fe1dcf |