Bug 117001

Summary: WebKit's Enter Full Screen for <video> elements isn't working
Product: WebKit Reporter: Ruth Fong <ruthiecftg>
Component: MediaAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, esprehn+autocc, glenn, jer.noble, jonlee, mitz, ruthiecftg, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Ruth Fong
Reported 2013-05-29 14:39:26 PDT
When hooking into WK's context menu action for entering into full screen for <video> elements, nothing happens (as opposed to hooking into WK's other media context menu actions, such as mute).
Attachments
Patch (1.27 KB, patch)
2013-05-30 12:05 PDT, Ruth Fong 		no flags
DetailsFormatted DiffDiff
Patch (1.61 KB, patch)
2013-05-30 16:45 PDT, Ruth Fong 		no flags
DetailsFormatted DiffDiff
Patch (1.99 KB, patch)
2013-05-30 17:13 PDT, Ruth Fong 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Ruth Fong
Comment 1 2013-05-29 14:41:58 PDT
<rdar://problem/14006095>
Ruth Fong
Comment 2 2013-05-30 12:05:25 PDT
Created attachment 203372 [details] Patch
Darin Adler
Comment 3 2013-05-30 12:31:56 PDT
Comment on attachment 203372 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=203372&action=review > Source/WebCore/ChangeLog:8 > + No new tests. Covered by existing tests for fullscreen. If this is covered by existing tests, why aren’t those tests failing?
Jer Noble
Comment 4 2013-05-30 12:59:58 PDT
Comment on attachment 203372 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=203372&action=review > Source/WebCore/dom/Document.cpp:4980 > + UserGestureIndicator indicator(DefinitelyProcessingNewUserGesture); > + By inserting this UserGestureIndicator, this patch would disable the security requirement that full screen requests must begin with an event handler. The appropriate place to put this line is in your context menu handler, immediately before calling element->webkitRequestFullScreen(), not inside requestFullScreenForElement() itself.
Ruth Fong
Comment 5 2013-05-30 16:45:27 PDT
Created attachment 203396 [details] Patch
Jer Noble
Comment 6 2013-05-30 16:55:11 PDT
Comment on attachment 203396 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=203396&action=review > Source/WebCore/ChangeLog:11 > + No new tests. > + > + * rendering/HitTestResult.cpp: > + (WebCore::HitTestResult::enterFullscreenForVideo): Please describe the problem which this patch is trying to solve, and why the approach here fixes the problem. You should also explain why no new tests were added: i.e, there is an existing test, media/context-menu-actions.html, but it is disabled due to bug #116651.
Ruth Fong
Comment 7 2013-05-30 17:13:07 PDT
Created attachment 203400 [details] Patch
WebKit Commit Bot
Comment 8 2013-05-30 19:31:52 PDT
Comment on attachment 203400 [details] Patch Clearing flags on attachment: 203400 Committed r151003: <http://trac.webkit.org/changeset/151003>
WebKit Commit Bot
Comment 9 2013-05-30 19:31:55 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.