Bug 35755

Summary: [Qt] QtLauncher needs an option to toggle FullScreen Mode
Product: WebKit Reporter: Jesus Sanchez-Palencia <jesus>
Component: Tools / TestsAssignee: QtWebKit Unassigned <webkit-qt-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, kent.hansen
Priority: P2 Keywords: Qt
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 35303    
Attachments:
Description Flags
Add FullScreen option to menu
ariya.hidayat: review-
Add FullScreen option to menu - v2
hausmann: review+, commit-queue: commit-queue-
Add FullScreen option to menu - v3 none

Description Jesus Sanchez-Palencia 2010-03-04 11:09:40 PST
Add an option to QtLauncher so we can toggle between FullScreen and default modes.
On Symbian, the default mode must be a Maximized window.
Comment 1 Jesus Sanchez-Palencia 2010-03-04 11:14:08 PST
Created attachment 50038 [details]
Add FullScreen option to menu
Comment 2 Ariya Hidayat 2010-03-04 11:34:43 PST
Comment on attachment 50038 [details]
Add FullScreen option to menu

Overall looks good.

> +signals:
> +    void fullscreen(bool on);

Usually signal name is future (aboutToDoFoo) or past (foobarHasChanged).

> +    // If click pos is the bottom right corner (square with size defined by gExitClickArea)
> +    // and the window is on FullScreen, the window must return to its original state.
> +    if (event->type() == QEvent::MouseButtonRelease) {
> +        QMouseEvent* ev = static_cast<QMouseEvent*>(event);
> +        if (windowState() == Qt::WindowFullScreen
> +            && ev->pos().x() > (size().width() - gExitClickArea)
> +            && ev->pos().y() > (size().height() - gExitClickArea)) {

Using width() and height() directly does not work?

> +    toggleFullScreen->connect(this, SIGNAL(fullscreen(bool)), SLOT(setChecked(bool)));

Is this a correct connection?
Comment 3 Jesus Sanchez-Palencia 2010-03-04 14:03:49 PST
Created attachment 50052 [details]
Add FullScreen option to menu - v2

Changes after Ariya's review.
Comment 4 Jesus Sanchez-Palencia 2010-03-04 14:06:02 PST
(In reply to comment #2)
> (From update of attachment 50038 [details])
> Overall looks good.
> 
> > +signals:
> > +    void fullscreen(bool on);
> 
> Usually signal name is future (aboutToDoFoo) or past (foobarHasChanged).

Ok, changed this.

> 
> > +    // If click pos is the bottom right corner (square with size defined by gExitClickArea)
> > +    // and the window is on FullScreen, the window must return to its original state.
> > +    if (event->type() == QEvent::MouseButtonRelease) {
> > +        QMouseEvent* ev = static_cast<QMouseEvent*>(event);
> > +        if (windowState() == Qt::WindowFullScreen
> > +            && ev->pos().x() > (size().width() - gExitClickArea)
> > +            && ev->pos().y() > (size().height() - gExitClickArea)) {
> 
> Using width() and height() directly does not work?

Yes, indeed! Fixed.

> 
> > +    toggleFullScreen->connect(this, SIGNAL(fullscreen(bool)), SLOT(setChecked(bool)));
> 
> Is this a correct connection?

Yes. This is to handle when you exit FullScreen mode by clicking on the "exit area" (bottom right corner), until we have a nice fullscreen button to go there.
I've added a comment to the patch so this won't confuse anyone in the future.

thanks!
Comment 5 WebKit Commit Bot 2010-03-07 00:25:09 PST
Comment on attachment 50052 [details]
Add FullScreen option to menu - v2

Rejecting patch 50052 from commit-queue.

Failed to run "['/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', '--reviewer', 'Simon Hausmann', '--force']" exit_code: 1
Last 500 characters of output:
ing file WebKitTools/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file WebKitTools/QtLauncher/main.cpp
Hunk #3 FAILED at 119.
Hunk #4 succeeded at 188 (offset 1 line).
Hunk #5 succeeded at 200 (offset 1 line).
Hunk #6 succeeded at 311 (offset 1 line).
Hunk #7 succeeded at 389 (offset 1 line).
Hunk #8 succeeded at 539 (offset 1 line).
Hunk #9 FAILED at 553.
Hunk #10 succeeded at 647 (offset 6 lines).
2 out of 10 hunks FAILED -- saving rejects to file WebKitTools/QtLauncher/main.cpp.rej

Full output: http://webkit-commit-queue.appspot.com/results/342785
Comment 6 Jesus Sanchez-Palencia 2010-03-08 09:52:15 PST
There was a problem with the patches order, I guess. I'll fix this as soon as I'm back from Bossa conference (in 4 days).
Comment 7 Tor Arne Vestbø 2010-03-10 06:43:39 PST
Please follow the QtWebKit bug reporting guidelines when reporting bugs.

See http://trac.webkit.org/wiki/QtWebKitBugs

Specifically:

  - The 'QtWebKit' component should only be used for bugs/features in the
    public QtWebKit API layer, not to signify that the bug is specific to
    the Qt port of WebKit

      http://trac.webkit.org/wiki/QtWebKitBugs#Component

  - Add the keyword 'Qt' to signal that it's a Qt-related bug

      http://trac.webkit.org/wiki/QtWebKitBugs#Keywords
Comment 8 Jesus Sanchez-Palencia 2010-03-11 13:49:22 PST
Created attachment 50534 [details]
Add FullScreen option to menu - v3

Fixed.
Comment 9 WebKit Commit Bot 2010-03-13 00:14:30 PST
Comment on attachment 50534 [details]
Add FullScreen option to menu - v3

Clearing flags on attachment: 50534

Committed r55951: <http://trac.webkit.org/changeset/55951>
Comment 10 WebKit Commit Bot 2010-03-13 00:14:35 PST
All reviewed patches have been landed.  Closing bug.