Bug 58007 – [Qt] Add -maximize flag to QtTestBrowser and MiniBrowser

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Keith Kyzivat]

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.

[Keith Kyzivat]

Created attachment 88558 [details]
Patch

[Keith Kyzivat]

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).

[Benjamin Poulain]

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.

[Benjamin Poulain]

(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.

[Keith Kyzivat]

Created attachment 88695 [details]
Patch

[Alexis Menard (darktears)]

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?

[Keith Kyzivat]

Created attachment 89223 [details]
Patch

[Keith Kyzivat]

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

[Keith Kyzivat]

Created attachment 89430 [details]
Patch

[Laszlo Gombos]

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.

[Keith Kyzivat]

Created attachment 89505 [details]
Patch

[Laszlo Gombos]

Comment on attachment 89505 [details]
Patch

r=me.

[WebKit Commit Bot]

Comment on attachment 89505 [details]
Patch

Clearing flags on attachment: 89505

Committed r83928: <http://trac.webkit.org/changeset/83928>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.