Bug 130603 - [iOS] Enable caption support in full screen.
Summary: [iOS] Enable caption support in full screen.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-03-21 10:19 PDT by Jer Noble
Modified: 2014-03-25 16:43 PDT (History)
6 users (show)

See Also:


Attachments
Patch (6.46 KB, patch)
2014-03-21 10:21 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (7.84 KB, patch)
2014-03-25 11:37 PDT, Jer Noble
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 Jer Noble 2014-03-21 10:19:35 PDT
[iOS] Enable caption support in full screen.
Comment 1 Jer Noble 2014-03-21 10:21:47 PDT
Created attachment 227450 [details]
Patch
Comment 2 Jer Noble 2014-03-25 11:37:04 PDT
Created attachment 227775 [details]
Patch
Comment 3 Eric Carlson 2014-03-25 13:53:41 PDT
Comment on attachment 227775 [details]
Patch

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

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:33
> +#include <wtf/WeakPtr.h>

Is this required?

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:814
> +        CGRect textFrame = m_videoLayer ? [m_videoLayer videoRect] : (CGRect)frame;

Can't you use static_cast<CGRect>() ?

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:1493
> +        CGRect frame = CGRectMake(0, 0, m_videoFullscreenFrame.width(), m_videoFullscreenFrame.height());

Nit: this local isn't really necessary.
Comment 4 Jer Noble 2014-03-25 15:39:02 PDT
(In reply to comment #3)
> (From update of attachment 227775 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=227775&action=review
> 
> > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:33
> > +#include <wtf/WeakPtr.h>
> 
> Is this required?

Nope.

Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:814
> > +        CGRect textFrame = m_videoLayer ? [m_videoLayer videoRect] : (CGRect)frame;
> 
> Can't you use static_cast<CGRect>() ?

Yep.

Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:1493
> > +        CGRect frame = CGRectMake(0, 0, m_videoFullscreenFrame.width(), m_videoFullscreenFrame.height());
> 
> Nit: this local isn't really necessary.

Sure ain't.
Comment 5 Jer Noble 2014-03-25 16:43:41 PDT
Committed r166268: <http://trac.webkit.org/changeset/166268>