Review Request 127216: [KStatusNotifierItem] MinimizeRestore does not "run" over the desktop on X11

[Thomas Lübking]

> On March 3, 2016, 10:16 p.m., Thomas Lübking wrote:
> > src/kstatusnotifieritem.cpp, line 934
> > <https://git.reviewboard.kde.org/r/127216/diff/2/?file=446044#file446044line934>
> >
> >     append
> >     associatedWidget->setAttribute(Qt::WA_Moved);
> >     
> >     
> >     
> >     This should tell Qt to demand an explicit position and skip placement by the WM (yes, this generally works in KWin ;-)
> >     
> >     However, this flag should be implicitly set by ::move() unless the point is invalid, so you might want to "qDebug() << associatedWidgetPos;" to check whether it's invalid for the failing clients.
> 
> Anthony Fieroni wrote:
>     Fun facts :) void KStatusNotifierItemPrivate::minimizeRestore(bool) is called only when you click on tray icon i.e. if i close app with big red X it's not called => i can't store position :)
> 
> Anthony Fieroni wrote:
>     More fun facts :) associatedWidget->setGeometry(associatedWidget->frameGeometry()); works in any case ! I will add path if you want, but you will not be happy to set geometry by myself :)
> 
> Anthony Fieroni wrote:
>     ^ No, extend geometry with decoration size :)
> 
> Thomas Lübking wrote:
>     meehhh...
>     
>     That means one will have to filter QEvent::Close of associatedWidget and store the position from there.
> 
> Anthony Fieroni wrote:
>     It's strange, about me, associatedWidget has correct frameGeometry even it's hide.
>     QObject::connect(KWindowSystem::self(), &KWindowSystem::windowRemoved, q, [this](WId winId) {
>             if (associatedWidget && associatedWidget->winId() == winId) {
>                 associatedWidgetPos = associatedWidget->frameGeometry().topLeft();
>             }
>         });
>     Position is correct, but again not work event with Qt::WA_Moved.
> 
> Anthony Fieroni wrote:
>     KWindowInfo info(associatedWidget->winId(), NET::WMDesktop | NET::WMGeometry);
>     info.geometry().topLeft(); <----------------------------- decoration is included again
>     to filed bug?
> 
> Thomas Lübking wrote:
>     You may file a bug, but I frankly doubt your findings.
>     (Just more or less ported my kwindowsystem tool and the values are correct ;-)
>     
>     => How exactly did you determine this assumption? (Just from the walking position?)
> 
> Anthony Fieroni wrote:
>     I can't move widget when new position not match frameGeometry().topLeft by itself.
>     associatedWidgetPos = associatedWidget->geometry().topLeft(); -> can move but new position is with decoration offset
>     associatedWidgetPos = associatedWidget->pos(); -> can't move, this is wanted position, not match internal wigdet frameGeometry
>     I saw qwidget move code, Qt::WA_Moved is setted at start.
>     I saw the qwidget close code, because when i click X -> now i CAN move to pos ! Why the hell, i try setAttribute(Qt::WA_QuitOnClose, false);
>     What are attribute setted/unsetted on X click?
> 
> Thomas Lübking wrote:
>     You'll fail to move because the call is idempotent, ie. the positions is assumed to be the current one and thus the move call is NOOP.
>     
>     Qt::WA_QuitOnClose exits the process when the last window closes, leave it alone.

Just tried, Quassel and trojitá (latter uses QSystrayIcon + QPA) actually work with the present code.
(Not checked details, but apparently minimizeRestore isn't invoked anyway??)

Trying amarok later (requires complete update for new libav deps) but would like to express my greatest disgrace for systrays in general and SNI in particular.
The SNI daemon silently quit (not killing kded!) several times! during the tests and I spent 5 minutes just to figure why I suddenly wasn't getting any tray icon and associated widgets turned 0x0 ... this entire thing is seriously far too fragile to be useful. *grrrr*


- Thomas


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


On Feb. 29, 2016, 5:18 a.m., Anthony Fieroni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127216/
> -----------------------------------------------------------
> 
> (Updated Feb. 29, 2016, 5:18 a.m.)
> 
> 
> Review request for KDE Frameworks, Martin Gräßlin, Thomas Lübking, and Martin Klapetek.
> 
> 
> Bugs: 356523
>     https://bugs.kde.org/show_bug.cgi?id=356523
> 
> 
> Repository: knotifications
> 
> 
> Description
> -------
> 
> Store position of widget before hide it
> 
> 
> Diffs
> -----
> 
>   src/kstatusnotifieritem.cpp cf3e7b5 
>   src/kstatusnotifieritemprivate_p.h 8fdfd4c 
> 
> Diff: https://git.reviewboard.kde.org/r/127216/diff/
> 
> 
> Testing
> -------
> 
> Tested on pixel ration = 1
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20160306/3aac2170/attachment.html>