Bug 61843 - Remove dependency on QTKit from wekitExitFullscreen()
Summary: Remove dependency on QTKit from wekitExitFullscreen()
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords:
Depends on:
Blocks: 61848
  Show dependency treegraph
 
Reported: 2011-06-01 01:06 PDT by Jer Noble
Modified: 2011-09-16 11:23 PDT (History)
4 users (show)

See Also:


Attachments
Patch (89.05 KB, patch)
2011-06-01 01:51 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (55.99 KB, patch)
2011-06-01 02:44 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
[PATCH 1/5] [LANDED] Move Full Screen Controllers into WebCore. (36.75 KB, patch)
2011-06-01 13:31 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
[PATCH 2/5] Add two new utility clasess: WebEventListener and WebMediaDelegate (27.61 KB, patch)
2011-06-01 13:31 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
[PATCH 3/5] WebVideoFullscreenController and WebVideoFullscreenHUDController now use a delegate (27.03 KB, patch)
2011-06-01 13:32 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
[PATCH 4/5] Disable calling into new full-screen API from video full-creen API. Enable video full-screen mode for AVFoundation. (7.69 KB, patch)
2011-06-01 13:32 PDT, Jer Noble
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2011-06-01 01:06:16 PDT
Remove dependency on QTKit from wekitExitFullscreen()
Comment 1 Jer Noble 2011-06-01 01:51:01 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.
Comment 2 Jer Noble 2011-06-01 02:44:19 PDT
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.
Comment 3 WebKit Review Bot 2011-06-01 02:48:27 PDT
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 4 Alexis Menard (darktears) 2011-06-01 04:24:23 PDT
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.
Comment 5 Jer Noble 2011-06-01 09:33:09 PDT
(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 6 Jer Noble 2011-06-01 13:17:44 PDT
Comment on attachment 95570 [details]
Patch

Clearing r?; I'll break this bug into smaller pieces and reattach.
Comment 7 Jer Noble 2011-06-01 13:31:17 PDT
Created attachment 95648 [details]
[PATCH 1/5] [LANDED] Move Full Screen Controllers into WebCore.
Comment 8 Jer Noble 2011-06-01 13:31:48 PDT
Created attachment 95649 [details]
[PATCH 2/5] Add two new utility clasess: WebEventListener and WebMediaDelegate
Comment 9 Jer Noble 2011-06-01 13:32:14 PDT
Created attachment 95650 [details]
[PATCH 3/5] WebVideoFullscreenController and  WebVideoFullscreenHUDController now use a delegate
Comment 10 Jer Noble 2011-06-01 13:32:41 PDT
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.
Comment 11 Jer Noble 2011-06-01 13:33:30 PDT
There is no 5/5. :-D
Comment 12 Eric Seidel 2011-06-02 08:38:36 PDT
Who should be CC'd to review these?
Comment 13 Alexis Menard (darktears) 2011-06-03 12:43:54 PDT
(In reply to comment #12)
> Who should be CC'd to review these?

Eric Carlson I believe.
Comment 14 Eric Carlson 2011-06-04 21:52:05 PDT
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.
Comment 15 Alexis Menard (darktears) 2011-06-20 11:30:23 PDT
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.
Comment 16 Jer Noble 2011-06-20 11:40:52 PDT
Patch 1/5 is blocking work on bug #61728, so I'll land that patch now.
Comment 17 Jer Noble 2011-06-20 12:29:48 PDT
Committed r89271: <http://trac.webkit.org/changeset/89271>
Comment 18 Jer Noble 2011-06-20 12:30:46 PDT
Whoops, re-opening.  Still have patches 2, 3, and 4 to review.
Comment 19 Jer Noble 2011-06-20 12:31:24 PDT
Comment on attachment 95648 [details]
[PATCH 1/5] [LANDED] Move Full Screen Controllers into WebCore.

Landed patch 1/5.  Clearing r+.
Comment 20 Jer Noble 2011-06-20 14:58:05 PDT
Committed a 32-bit build fix for patch 1/5.
 Committed r89292: <http://trac.webkit.org/changeset/89292>
Comment 21 Jer Noble 2011-06-20 15:23:23 PDT
Committed a build fix for Leopard.
Committed r89296: <http://trac.webkit.org/changeset/89296>
Comment 22 Jer Noble 2011-09-16 11:22:18 PDT
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".