Summary: | [Qt] Add -maximize flag to QtTestBrowser and MiniBrowser | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Keith Kyzivat <kamaji> | ||||||||||||
Component: | Tools / Tests | Assignee: | 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
Keith Kyzivat
2011-04-06 19:17:34 PDT
Created attachment 88558 [details]
Patch
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 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. (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. Created attachment 88695 [details]
Patch
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? Created attachment 89223 [details]
Patch
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 Created attachment 89430 [details]
Patch
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. Created attachment 89505 [details]
Patch
Comment on attachment 89505 [details]
Patch
r=me.
Comment on attachment 89505 [details] Patch Clearing flags on attachment: 89505 Committed r83928: <http://trac.webkit.org/changeset/83928> All reviewed patches have been landed. Closing bug. |