Bug 58007

Summary: [Qt] Add -maximize flag to QtTestBrowser and MiniBrowser
Product: WebKit Reporter: Keith Kyzivat <kamaji>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Enhancement CC: benjamin, commit-queue, cshu, laszlo.gombos, ossy, yael
Priority: P3 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Keith Kyzivat 2011-04-06 19:17:34 PDT
Small devices like phones and PDAs really need to run their apps maximized or fullscreen.

I propose that a -maximized paramater be added to these test applications.
Comment 1 Keith Kyzivat 2011-04-06 19:22:39 PDT
Created attachment 88558 [details]
Patch
Comment 2 Keith Kyzivat 2011-04-06 19:27:29 PDT
An interesting thing I noticed when implementing this.

On Desktop Linux, if you pass -maximized to MiniBrowser, the window will indeed start maximized, but the area rendered is small.
If you then alter the window size, it will properly render.
If you then maximize it with window controls, it will also render properly.

I was able to correct it with:
resize(QApplication::desktop()->screenGeometry().size());
before the setWindowState(Qt::WindowMaximized), however that just masks the problem, so I left it out.

It works properly on QtTestBrowser, and on MiniBrowser in other contexts (without X11).
Comment 3 Benjamin Poulain 2011-04-07 03:15:01 PDT
Comment on attachment 88558 [details]
Patch

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

> Tools/QtTestBrowser/launcherwindow.cpp:791
> -        setWindowState(Qt::WindowNoState);
> +        if (m_windowOptions.preferMaximized)
> +            setWindowState(Qt::WindowMaximized);
> +        else
> +            setWindowState(Qt::WindowNoState);

I think the window should come back in its previous state when going out of fullscreen.

> Tools/QtTestBrowser/launcherwindow.cpp:883
> +    if (m_windowOptions.preferMaximized)
> +        dialog->setWindowState(Qt::WindowMaximized);
> +    else
> +        dialog->resize(size().width() * 0.7, dialog->size().height());

This is a dialog, that should not get maximized by default. The window manager should decide how to handle dialogs.

> Tools/QtTestBrowser/main.cpp:186
> +             << "[-maximized"

Missing ]


Same comments for both apps.
Comment 4 Benjamin Poulain 2011-04-07 03:15:45 PDT
(In reply to comment #2)
> An interesting thing I noticed when implementing this.
> 
> On Desktop Linux, if you pass -maximized to MiniBrowser, the window will indeed start maximized, but the area rendered is small.
> If you then alter the window size, it will properly render.
> If you then maximize it with window controls, it will also render properly.
> 
> I was able to correct it with:
> resize(QApplication::desktop()->screenGeometry().size());
> before the setWindowState(Qt::WindowMaximized), however that just masks the problem, so I left it out.
> 
> It works properly on QtTestBrowser, and on MiniBrowser in other contexts (without X11).

Please make a reduction and open a bug for that so we don't forget to fix it.
Comment 5 Keith Kyzivat 2011-04-07 14:23:00 PDT
Created attachment 88695 [details]
Patch
Comment 6 Alexis Menard (darktears) 2011-04-12 05:48:41 PDT
Comment on attachment 88695 [details]
Patch

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

> Tools/MiniBrowser/qt/BrowserWindow.cpp:326
> +

Why this blank line?
Comment 7 Keith Kyzivat 2011-04-12 10:42:16 PDT
Created attachment 89223 [details]
Patch
Comment 8 Keith Kyzivat 2011-04-12 10:45:51 PDT
Fixed leftover blank line from older patch.
Also - I provided a reduction for the bug that is mentioned in comment #2 and #4 - you can find this reported as bug 58152
Comment 9 Keith Kyzivat 2011-04-13 12:16:18 PDT
Created attachment 89430 [details]
Patch
Comment 10 Laszlo Gombos 2011-04-13 15:05:18 PDT
Comment on attachment 89430 [details]
Patch

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

Please fix the minor comment on the SYMBIAN section; otherwise looks good to me.

> Tools/QtTestBrowser/launcherwindow.cpp:71
>  #if defined(Q_OS_SYMBIAN)
>      setWindowState(Qt::WindowMaximized);
>  #else

Perhaps you should move this special case for SYMBIAN to where we process the -maximize option. Please do that for the MiniBrowser as well.
Comment 11 Keith Kyzivat 2011-04-13 17:53:35 PDT
Created attachment 89505 [details]
Patch
Comment 12 Laszlo Gombos 2011-04-14 04:41:52 PDT
Comment on attachment 89505 [details]
Patch

r=me.
Comment 13 WebKit Commit Bot 2011-04-14 18:44:24 PDT
Comment on attachment 89505 [details]
Patch

Clearing flags on attachment: 89505

Committed r83928: <http://trac.webkit.org/changeset/83928>
Comment 14 WebKit Commit Bot 2011-04-14 18:44:31 PDT
All reviewed patches have been landed.  Closing bug.