Summary: | Remove dependency on QTKit from wekitExitFullscreen() | ||
---|---|---|---|
Product: | WebKit | Reporter: | Jer Noble <jer.noble> |
Component: | WebKit Misc. | Assignee: | Jer Noble <jer.noble> |
Status: | RESOLVED WONTFIX | ||
Severity: | Normal | CC: | eric.carlson, eric, menard, webkit.review.bot |
Priority: | P2 | ||
Version: | 528+ (Nightly build) | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
Bug Depends on: | |||
Bug Blocks: | 61848 | ||
Attachments: |
Description
Jer Noble
2011-06-01 01:06:16 PDT
Created attachment 95567 [details]
Patch
This is the first bug of two. The second one will build upon this one to implement the video full-screen API in WebKit2.
Created attachment 95570 [details]
Patch
Discovered a small problem with the volume slider; it was scaling between 0-100, and the previous patch was setting it to 0-1.
Attachment 95570 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebKit/mac/WebView/WebMediaDelegate.h:32: Code inside a namespace should not be indented. [whitespace/indent] [4]
Total errors found: 1 in 18 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 95570 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95570&action=review Damn this patch will badly break https://bugs.webkit.org/show_bug.cgi?id=61728 > Source/WebCore/ChangeLog:106 > + * platform/mac/WebWindowAnimation.h: Renamed from Source/WebKit/mac/WebView/WebWindowAnimation.h. There is a previous patch doing almost that waiting for review https://bugs.webkit.org/show_bug.cgi?id=61522. Perhaps we should cancel it. (In reply to comment #4) > (From update of attachment 95570 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=95570&action=review > > Damn this patch will badly break https://bugs.webkit.org/show_bug.cgi?id=61728 Yes, it probably will. However, it looks like QTKitFullScreenVideoHandler could just create an instance of the new WebMediaHandler class to pass along to the fullscreen controller (instead of passing an HTMLElement directly). Apart from that, and different include paths for WebVideoFullscreenController.h, nothing here should be insurmountable. > > Source/WebCore/ChangeLog:106 > > + * platform/mac/WebWindowAnimation.h: Renamed from Source/WebKit/mac/WebView/WebWindowAnimation.h. > > There is a previous patch doing almost that waiting for review > https://bugs.webkit.org/show_bug.cgi?id=61522. Perhaps we should cancel it. Lets see what happens to this bug first. It may get roundly rejected. :) Comment on attachment 95570 [details]
Patch
Clearing r?; I'll break this bug into smaller pieces and reattach.
Created attachment 95648 [details]
[PATCH 1/5] [LANDED] Move Full Screen Controllers into WebCore.
Created attachment 95649 [details]
[PATCH 2/5] Add two new utility clasess: WebEventListener and WebMediaDelegate
Created attachment 95650 [details]
[PATCH 3/5] WebVideoFullscreenController and WebVideoFullscreenHUDController now use a delegate
Created attachment 95651 [details]
[PATCH 4/5] Disable calling into new full-screen API from video full-creen API. Enable video full-screen mode for AVFoundation.
There is no 5/5. :-D Who should be CC'd to review these? (In reply to comment #12) > Who should be CC'd to review these? Eric Carlson I believe. Comment on attachment 95648 [details] [PATCH 1/5] [LANDED] Move Full Screen Controllers into WebCore. View in context: https://bugs.webkit.org/attachment.cgi?id=95648&action=review > Source/WebCore/WebCore.exp.in:1371 > _wkMeasureMediaUIPart > +_wkCreateMediaUIBackgroundView > +_wkCreateMediaUIControl > +_wkWindowSetAlpha > +_wkWindowSetScaledFrame > _wkMediaControllerThemeAvailable It looks like exports in this file should be in sort order when not inside #if defs. Jer could you land patch 1, so I can rebase my patch, it's quite needed for Qt? It seems that patch 2,3 and 4 are not related to us. Thanks. Patch 1/5 is blocking work on bug #61728, so I'll land that patch now. Committed r89271: <http://trac.webkit.org/changeset/89271> Whoops, re-opening. Still have patches 2, 3, and 4 to review. Comment on attachment 95648 [details]
[PATCH 1/5] [LANDED] Move Full Screen Controllers into WebCore.
Landed patch 1/5. Clearing r+.
Committed a 32-bit build fix for patch 1/5. Committed r89292: <http://trac.webkit.org/changeset/89292> Committed a build fix for Leopard. Committed r89296: <http://trac.webkit.org/changeset/89296> This bug is no longer being actively pursued. Clearing the r? flags from the outstanding patches and marking this bug as "not to be fixed". |