[KDE/Mac] Review Request 126324: [MSWin/OS X] save and restore window geometry instead of only size (WIP/Suggestion)

Thu Dec 17 17:21:48 UTC 2015

> On Dec. 17, 2015, 5:16 p.m., Martin Gräßlin wrote:
> > src/gui/kwindowconfig.h, lines 38-39
> > <https://git.reviewboard.kde.org/r/126324/diff/4/?file=422749#file422749line38>
> >
> >     That doesn't match the method name. It's saveWindowSize, not saveWindowGeometry. It's highly unexpected that saveWindowSize saves the position.
> >     
> >     If you want that: please introduce a new saveWindowGeometry method.

I was afraid someone was going to say that, which is why I tried to argue that it's highly unexpected from a user viewpoint that only window size is saved and not position. How often would it happen that a developer is "highly surprised" in a *negative* way that window size AND position are restored on a platform where this is the default behaviour?

I have nothing against introducing a pair of new methods, but how is that supposed to be done in transparent fashion? I do have a lot against a need to change all dependent software to call those methods (maintenance burden and all that).

Counter proposal: replace save/restoreWindowSize with save/restoreWindowGeometry everywhere, with a platform-specific interpretation of what exactly geometry encompasses. Much less surprise there, just a bit more need to read the documentation. Are these functions ever called intentionally outside of what I suppose is a more or less automatic feature that takes care of restoring window, erm, layout (saving is clearly automatic).

- René J.V.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126324/#review89665
-----------------------------------------------------------

On Dec. 14, 2015, 5:04 p.m., René J.V. Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126324/
> -----------------------------------------------------------
> 
> (Updated Dec. 14, 2015, 5:04 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and KDE Frameworks.
> 
> 
> Repository: kconfig
> 
> 
> Description
> -------
> 
> In KDElibs4, the KMainWindow::saveWindowSize() and KMainWindow::restoreWindowSize() function saved and restored not only the size but also the position (i.e. the geometry) of windows, using QWidget::saveGeometry and QWidget::restoreGeometry.
> 
> 2 main reasons for this (according to the comments):
> - Under X11 restoring the position is tricky
> - X11 has a window manager which might be considered responsible for that functionality (and I suppose most modern WMs have the feature enabled by default?)
> 
> Both arguments are moot on MS Windows and OS X, and on both platforms users expect to see window positions restored as well as window size. On OS X there is also little choice in the matter: most applications offer the geometry restore without asking (IIRC it is the same on MS Windows).
> 
> I would thus like to propose to port the platform-specific code that existed for MS Windows (and for OS X as a MacPorts patch that apparently was never submitted upstreams). I realise that this violates the message conveyed by the function names but I would like to think that this is a case where function is more important.
> 
> You may also notice that the Mac version does not store resolution-specific settings. This happens to work best on OS X, where multi-screen support has been present since the early nineties, and where window geometry is restored regardless of the screen resolution (i.e. connect a different external screen with a different resolution, and windows will reopen as they were on that screen, not with some default geometry).
> I required I can update the comments in the header to reflect this subtlety.
> 
> Note that for optimal functionality a companion patch to `KMainWindow::event` is required:
> ```
> --- a/src/kmainwindow.cpp
> +++ b/src/kmainwindow.cpp
> @@ -772,7 +772,7 @@ bool KMainWindow::event(QEvent *ev)
>  {
>      K_D(KMainWindow);
>      switch (ev->type()) {
> -#ifdef Q_OS_WIN
> +#if defined(Q_OS_WIN) || defined(Q_OS_OSX)
>      case QEvent::Move:
>  #endif
>      case QEvent::Resize:
> ```
> 
> This ensures that the window geometry save is performed also after a move (to update the position) without requiring a dummy resizing operation.
> Do I need to create a separate RR for this change or is it small enough that I can push it if and when this RR is accepted?
> 
> 
> Diffs
> -----
> 
>   src/gui/kwindowconfig.h 48a8f3c 
>   src/gui/kwindowconfig.cpp d2f355c 
> 
> Diff: https://git.reviewboard.kde.org/r/126324/diff/
> 
> 
> Testing
> -------
> 
> On OS X 10.6 through 10.9 with various KDElibs4 versions and now with Qt 5.5.1 and frameworks 5.16.0 (and Kate as a test application).
> I presume that the MS Windows code has been tested sufficiently in KDELibs4; I have only adapted it to Qt5 and tested if it builds.
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-mac/attachments/20151217/78702b51/attachment-0001.html>