Review Request: use _NET_WM_STATE_HIDDEN to check if the window is minimized instead of WM_STATE == ICONIC when possible.

[Thomas Lübking]

> On Jan. 9, 2013, 10:45 p.m., Thomas Lübking wrote:
> > Random addendums:
> > - state hidden is also provided by metacity, icewm, openbox and compiz
> > - mappingState() is part of the public API, thus not cut off (if you actually want to know it)
> > - not unmapping windows has more issues than the pager/taskbar, so it's not like minimization wasn't still "broken" by the setting (non-kde/clients relying on rather the actual mapping state to stall output processing etc.)
> 
> Yichao Yu wrote:
>     (well I am not talking about removing mappingState() ....... k, good to know it is public :P...)
>     
>     Yes I am aware of those issues by not unmapping windows (from bug report here[1], and I don't think I am using a application that have those problems), but I don't think isMinimized() does it correctly right now.
>     
>     [1] https://bugs.kde.org/show_bug.cgi?id=189435
>
> 
> Thomas Lübking wrote:
>     This was not meant as criticism to the patch but only "things others might immediately worry about as well" - so they don't have to check them.
>     
>     What needed to be tested is for what occasions other WMs set the iconic state (which becomes a shortcut "true" with the patch)
>     Could be a problem with virtual desktop or shading implementations.
>     
>     In this case a less invasive alternative would be to alter the taskbar/pager implementation to check KWindowInfo::state() rather then ::isMinimized()
>     
>     I'll test the ones mentioned above and later on e17
> 
> Xuetian Weng wrote:
>     ... ahhh, while I'm discussing with Yichao Yu I didn't realize he also submit a patch.. 
>     
>     but logically this patch has problem.
>     
>     If wm doesn't support netwm, and current mappingState is iconic. This code will get things wrong for non-minimized window.
>     first if: false, since window is not set to Iconic
>     second if: false, since wm is netwm 1.2 compatible
>     return: finally return true, since it's not netwm 1.2 compatible, while it should be false.
> 
> Xuetian Weng wrote:
>     ah sorry, a typo: and current mappingState is NOT iconic.

Yes, it's pretty much the showstopper for this patch (sorry)

Metacity, Compiz and Openbox set the iconified state for windows on different virtual desktops or being shaded.
IceWM and KWin don't - i've not tested e17 (doesn't matter anymore)

While this can be tested by KWindowSystem::icccmCompliantMappingState(), that doesn't help the patch as it is (we'd have to prefer the NETWM hint over ICCCM)

Reg. not NETWM compliant WMs - i've still mwm around, but it doesn't reasonably work at all ;-)


- Thomas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108308/#review25103
-----------------------------------------------------------


On Jan. 9, 2013, 9:06 p.m., Yichao Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108308/
> -----------------------------------------------------------
> 
> (Updated Jan. 9, 2013, 9:06 p.m.)
> 
> 
> Review request for kdelibs, kwin and Aaron J. Seigo.
> 
> 
> Description
> -------
> 
> When setting "Keep window thumbnails" to "Always (Breaks minimization)", kwin will keep WM_STATE to be NORMAL when a client is minimized while including _NET_WM_STATE_HIDDEN in its _NET_WM_STATE, as confirmed by ICCCM[1] and Extended Window Manager Hints[2]. However, apart from the expected result (breaks minimization: the client will continue to refresh its content) the minimized window is not shown as minimized in icontasks and pager.
> 
> These two plasma addons (and probably other addons as well) uses KWindowInfo::isMinimized to determine whether the window is minimized. However, this function threat all window that are not Iconic (WM_STATE != ICONIC) as not minimized, in contradiction to the "Extended window manager hints" which says, "Pagers and similar applications should use _NET_WM_STATE_HIDDEN instead of WM_STATE to decide whether to display a window in miniature representations of the windows on a desktop."
> 
> This patch correct this behavior and therefore correct the behavior of both pager and icontasks in this situation.
> 
> [1] http://tronche.com/gui/x/icccm/sec-4.html#s-4.1.3.1
> [2] http://standards.freedesktop.org/wm-spec/wm-spec-1.3.html#id2731936
> 
> 
> Diffs
> -----
> 
>   kdeui/windowmanagement/kwindowinfo_x11.cpp d983c9a 
> 
> Diff: http://git.reviewboard.kde.org/r/108308/diff/
> 
> 
> Testing
> -------
> 
> Compiled, pager and icontasks shows minimized windows correctly.
> 
> 
> Thanks,
> 
> Yichao Yu
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20130109/4ba84fd7/attachment.htm>