Bug 54201 – [Qt] In trunk with Qt Multimedia the full screen mode doesn't work.

• Component: Media
• Status: RESOLVED FIXED

[Alexis Menard (darktears)]

When browsing website with HTML5 video tag, there is no way to go to fullscreen. The button doesn't appear.

[Alexis Menard (darktears)]

Created attachment 81966 [details]
Patch to implement a default fullscreen handler...

Add a default handler for the fullscreen playback when there is no platform plugin install.

[WebKit Review Bot]

Attachment 81966 [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/qt/WebCoreSupport/FullScreenVideoWidget.cpp:28:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 1 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Andreas Kling]

Comment on attachment 81966 [details]
Patch to implement a default fullscreen handler...

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

> Source/WebKit/qt/WebCoreSupport/FullScreenVideoWidget.cpp:36
> +static const int hideMouseCursorDelay = 3000;

gHideMouseCursorDelay

> Source/WebKit/qt/WebCoreSupport/FullScreenVideoWidget.cpp:44
> +    QPalette palette;

Shouldn't we be amending the existing palette() here?

> Source/WebKit/qt/WebCoreSupport/FullScreenVideoWidget.cpp:67
> +    connect(&m_cursorTimer, SIGNAL(timeout()), this, SLOT(hideCursor()));

Multiple calls to show() will cause signal/slot connections to accumulate.
You can pass Qt::UniqueConnection as the 5th argument to avoid this.

> Source/WebKit/qt/WebCoreSupport/FullScreenVideoWidget.cpp:108
> +    } else if (ev->key() == Qt::Key_Escape) {
> +        close();
> +    }

Coding style, no { } for single-statement blocks.

> Source/WebKit/qt/WebCoreSupport/FullScreenVideoWidget.h:44
> +    ~FullScreenVideoWidget();

virtual ~FullScreenVideoWidget();

> Source/WebKit/qt/WebCoreSupport/FullScreenVideoWidget.h:55
> +protected slots:

Might as well be private.

[Alexis Menard (darktears)]

Created attachment 81981 [details]
Patch to implement a default fullscreen handler... v2

Andreas' comments taken into account.

[WebKit Review Bot]

Attachment 81981 [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/qt/WebCoreSupport/FullScreenVideoWidget.cpp:28:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 1 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Andreas Kling]

Comment on attachment 81981 [details]
Patch to implement a default fullscreen handler... v2

r=me

[WebKit Commit Bot]

Comment on attachment 81981 [details]
Patch to implement a default fullscreen handler... v2

Clearing flags on attachment: 81981

Committed r78240: <http://trac.webkit.org/changeset/78240>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.