Bug 246442

Summary: Close window control missing for application under Wine
Product: [Plasma] kwin Reporter: Andreas Hermann Braml <andreas>
Component: generalAssignee: KWin default assignee <kwin-bugs-null>
Status: RESOLVED FIXED    
Severity: normal    
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: Ubuntu   
OS: Linux   
See Also: https://bugs.kde.org/show_bug.cgi?id=347818
Latest Commit: Version Fixed In: 4.8.1
Attachments: testcase

Description Andreas Hermann Braml 2010-08-01 18:32:37 UTC
Version:           unspecified (using KDE 4.4.92) 
OS:                Linux

I run an application under Wine 1.2 that starts a main window in the background, another one in the foreground that has the focus. The expected behavior is:

1) as long as the window in the foreground is present, the background window either has no close window control (the 'x' in the corner) or it is shown, but does nothing. Metacity, the WM of Fluxbox either do the one thing or the other.

2) as soon as the window in the foreground closes after entering some data there, the background window should get the focus and the close button should appear/become functional.

Alas KWin doesn't behave like this. On KWin, the steps are as follows:

1) as long as the window in the foreground is present, the background window has no close window control (nothing wrong so far)

2) as soon as the window in the foreground closes, neither does the background window get focus nor does the 'x' appear

Reproducible: Always

Steps to Reproduce:
- install TREPCAD 3.4.0 on Wine 1.2 (or any other version after Wine GIT commit e35e75b4bf5db54cd6cda0c95dc8c9e14c1fccfb
- run it with 'wine trepcad.exe'
- fill out the form in the foreground window, then click OK

Actual Results:  
the close control 'x' on the main window doesn't show up

Expected Results:  
there should be a close control on the main window after the window in the foreground window closes

For the triaging done so far see the bug report over at Wine http://bugs.winehq.org/show_bug.cgi?id=23180
Comment 1 Thomas Lübking 2010-08-01 18:45:33 UTC
since this issue is triggered by a commercial windows application, you'll have to dump the output of "xprop" on the window in question, 

*before
*during and
*after

causing what seems to be a modal dialog - and on this
*"foreground window"

as well to raise chances this can be debugged at all (just tested with notepad.exe as suggested in the wine report, no problem there)
Comment 2 Martin Flöser 2010-08-01 18:48:13 UTC
given comment http://bugs.winehq.org/show_bug.cgi?id=23180#c19 it looks like a 
wine bug.

Unfortunatelly I cannot reproduce, as I
* won't install wine (out of principle)
* won't install any random windows software

So I can only comment on the behavior. The removal of the close button sounds 
like _NET_WM_ACTION_CLOSE is removed from _NET_WM_ALLOWED_ACTIONS (you can 
verify this with xprop and clicking on the window. When the modal window 
closes this action should be set again and the button should be there again. 
You can verify this in the same way.

In general it's of course completely wrong to remove the close button when a 
modal dialog opens. A modal dialog blocks the closing of the main window. But 
this is wine and they had to implement this strange API. It sounds like a 
completely broken behavior.

To workaround your problem you can use a window specific rule to force the 
close button to the window.
Comment 3 Andreas Hermann Braml 2010-08-02 16:18:52 UTC
As requested per comment #1 , here's the xprop output for the app under Wine.

1) for the main window:
1a) when modal window is still there:
WM_STATE(WM_STATE):
                window state: Normal
                icon window: 0x0
_NET_WM_ALLOWED_ACTIONS(ATOM) = _NET_WM_ACTION_MINIMIZE, _NET_WM_ACTION_SHADE, _NET_WM_ACTION_MAXIMIZE_VERT, _NET_WM_ACTION_MAXIMIZE_HORZ, _NET_WM_ACTION_FULLSCREEN, _NET_WM_ACTION_CHANGE_DESKTOP
_KDE_NET_WM_FRAME_STRUT(CARDINAL) = 3, 3, 23, 4
_NET_FRAME_EXTENTS(CARDINAL) = 3, 3, 23, 4
_NET_WM_DESKTOP(CARDINAL) = 0
_NET_WM_VISIBLE_ICON_NAME(UTF8_STRING) = 0x54, 0x52, 0x45, 0x50, 0x43, 0x41, 0x44, 0x20, 0x53, 0x74, 0x20, 0x33, 0x20, 0x3c, 0x32, 0x3e, 0xe2, 0x80, 0x8e
_NET_WM_VISIBLE_NAME(UTF8_STRING) = 0x54, 0x52, 0x45, 0x50, 0x43, 0x41, 0x44, 0x20, 0x53, 0x74, 0x20, 0x33, 0x20, 0x3c, 0x32, 0x3e, 0xe2, 0x80, 0x8e
_KDE_NET_WM_USER_CREATION_TIME(CARDINAL) = 2573175
_NET_WM_STATE(ATOM) = _NET_WM_STATE_MAXIMIZED_VERT, _NET_WM_STATE_MAXIMIZED_HORZ
_NET_WM_NAME(UTF8_STRING) = 0x54, 0x52, 0x45, 0x50, 0x43, 0x41, 0x44, 0x20, 0x53, 0x74, 0x20, 0x33
WM_ICON_NAME(STRING) = "TREPCAD St 3"
WM_NAME(STRING) = "TREPCAD St 3"
WM_HINTS(WM_HINTS):
                Client accepts input or input focus: False
                Initial state is Normal State.
                bitmap id # to use for icon: 0x5000371
                bitmap id # of mask for icon: 0x5000377
                window id # of group leader: 0x5400001
_MOTIF_WM_HINTS(_MOTIF_WM_HINTS) = 0x3, 0x6, 0x7e, 0x7e758ff4, 0x32eee8
_NET_WM_WINDOW_TYPE(ATOM) = _NET_WM_WINDOW_TYPE_NORMAL
WM_NORMAL_HINTS(WM_SIZE_HINTS):
                window gravity: Static
WM_TRANSIENT_FOR(WINDOW): window id # 0x5400001
_NET_WM_ICON(CARDINAL) =        Icon (32 x 32):

 [--snip--]

_NET_WM_USER_TIME_WINDOW(WINDOW): window id # 0x500003b
XdndAware(ATOM) = ATOM
_NET_WM_PID(CARDINAL) = 5254
WM_LOCALE_NAME(STRING) = "de_DE.UTF-8"
WM_CLIENT_MACHINE(STRING) = "andi-desktop"
WM_CLASS(STRING) = "trepcad.exe", "Wine"
WM_PROTOCOLS(ATOM): protocols  WM_DELETE_WINDOW, _NET_WM_PING, WM_TAKE_FOCUS


1b) after the modal window has closed:
WM_STATE(WM_STATE):
                window state: Normal
                icon window: 0x0
_NET_WM_ALLOWED_ACTIONS(ATOM) = _NET_WM_ACTION_MINIMIZE, _NET_WM_ACTION_SHADE, _NET_WM_ACTION_MAXIMIZE_VERT, _NET_WM_ACTION_MAXIMIZE_HORZ, _NET_WM_ACTION_FULLSCREEN, _NET_WM_ACTION_CHANGE_DESKTOP, _NET_WM_ACTION_CLOSE
_KDE_NET_WM_FRAME_STRUT(CARDINAL) = 3, 3, 23, 4
_NET_FRAME_EXTENTS(CARDINAL) = 3, 3, 23, 4
_NET_WM_DESKTOP(CARDINAL) = 0
_KDE_NET_WM_USER_CREATION_TIME(CARDINAL) = 2573175
_NET_WM_STATE(ATOM) = _NET_WM_STATE_MAXIMIZED_VERT, _NET_WM_STATE_MAXIMIZED_HORZ
_NET_WM_NAME(UTF8_STRING) = 0x73, 0x74, 0x6e, 0x64, 0x5f, 0x31, 0x78, 0x34, 0x6c, 0x2e, 0x74, 0x72, 0x73, 0x20, 0x2d, 0x20, 0x54, 0x52, 0x45, 0x50, 0x43, 0x41, 0x44, 0x20, 0x53, 0x74, 0x20, 0x33, 0x20, 0x2d, 0x20, 0x5b, 0x4b, 0x6f, 0x6e, 0x73, 0x74, 0x72, 0x75, 0x6b, 0x74, 0x69, 0x6f, 0x6e, 0x73, 0x62, 0x65, 0x72, 0x65, 0x69, 0x63, 0x68, 0x5d
WM_ICON_NAME(STRING) = "stnd_1x4l.trs - TREPCAD St 3 - [Konstruktionsbereich]"
WM_NAME(STRING) = "stnd_1x4l.trs - TREPCAD St 3 - [Konstruktionsbereich]"
WM_HINTS(WM_HINTS):
                Client accepts input or input focus: False
                Initial state is Normal State.
                bitmap id # to use for icon: 0x5000371
                bitmap id # of mask for icon: 0x5000377
                window id # of group leader: 0x5400001
_MOTIF_WM_HINTS(_MOTIF_WM_HINTS) = 0x3, 0x3e, 0x7e, 0x7e758ff4, 0x32f584
_NET_WM_WINDOW_TYPE(ATOM) = _NET_WM_WINDOW_TYPE_NORMAL
WM_NORMAL_HINTS(WM_SIZE_HINTS):
                window gravity: Static
WM_TRANSIENT_FOR(WINDOW): window id # 0x5400001
_NET_WM_ICON(CARDINAL) =        Icon (32 x 32):

[--snip--]

_NET_WM_USER_TIME_WINDOW(WINDOW): window id # 0x500003b
XdndAware(ATOM) = ATOM
_NET_WM_PID(CARDINAL) = 5254
WM_LOCALE_NAME(STRING) = "de_DE.UTF-8"
WM_CLIENT_MACHINE(STRING) = "andi-desktop"
WM_CLASS(STRING) = "trepcad.exe", "Wine"
WM_PROTOCOLS(ATOM): protocols  WM_DELETE_WINDOW, _NET_WM_PING, WM_TAKE_FOCUS


Unfortunately, there's no state "before the modal window is there", as the app comes up with the modal window already present.


2) for the modal window:
WM_STATE(WM_STATE):
                window state: Normal
                icon window: 0x0
_NET_WM_ALLOWED_ACTIONS(ATOM) = _NET_WM_ACTION_MOVE, _NET_WM_ACTION_MINIMIZE, _NET_WM_ACTION_SHADE, _NET_WM_ACTION_CHANGE_DESKTOP
_KDE_NET_WM_FRAME_STRUT(CARDINAL) = 3, 3, 23, 4
_NET_FRAME_EXTENTS(CARDINAL) = 3, 3, 23, 4
_NET_WM_DESKTOP(CARDINAL) = 0
_NET_WM_STATE(ATOM) = _NET_WM_STATE_SKIP_TASKBAR
_KDE_NET_WM_USER_CREATION_TIME(CARDINAL) = 2573193
_NET_WM_NAME(UTF8_STRING) = 0x4c, 0x69, 0x7a, 0x65, 0x6e, 0x7a, 0x69, 0x6e, 0x66, 0x6f, 0x72, 0x6d, 0x61, 0x74, 0x69, 0x6f, 0x6e
WM_ICON_NAME(STRING) = "Lizenzinformation"
WM_NAME(STRING) = "Lizenzinformation"
WM_HINTS(WM_HINTS):
                Client accepts input or input focus: False
                Initial state is Normal State.
                bitmap id # to use for icon: 0x5000488
                bitmap id # of mask for icon: 0x500048e
                window id # of group leader: 0x5400006
_MOTIF_WM_HINTS(_MOTIF_WM_HINTS) = 0x3, 0x4, 0xa, 0x7e758ff4, 0x32eb50
_NET_WM_WINDOW_TYPE(ATOM) = _NET_WM_WINDOW_TYPE_DIALOG
WM_NORMAL_HINTS(WM_SIZE_HINTS):
                program specified location: 476, 306
                program specified minimum size: 487 by 272
                program specified maximum size: 487 by 272
                window gravity: Static
WM_TRANSIENT_FOR(WINDOW): window id # 0x5400006
_NET_WM_ICON(CARDINAL) =        Icon (32 x 32):

[--snip--]

_NET_WM_USER_TIME_WINDOW(WINDOW): window id # 0x500003b
XdndAware(ATOM) = ATOM
_NET_WM_PID(CARDINAL) = 5254
WM_LOCALE_NAME(STRING) = "de_DE.UTF-8"
WM_CLIENT_MACHINE(STRING) = "andi-desktop"
WM_CLASS(STRING) = "trepcad.exe", "Wine"
WM_PROTOCOLS(ATOM): protocols  WM_DELETE_WINDOW, _NET_WM_PING, WM_TAKE_FOCUS

Re comment #2:
Since Metacity and every other WM I try to trigger this bug on can handle this "completely broken behaviour" in a more or less sane way, there must have been many broken modular apps out there long before Wine started doing it "the wrong way" recently ;) Don't ask me as to why the other WMs cater for this brokenness, though.
Comment 4 Thomas Lübking 2010-08-02 16:56:33 UTC
The window removes and adds _NET_WM_ACTION_CLOSE

While this is still "wrong" behaviour (it's not the clients job to define the behaviour of modal windows and their transients - tagging them transient is sufficient), the observed problem is a KWin bug, the netwm spec says about _NET_WM_ALLOWED_ACTIONS:
"The Window Manager MUST keep this property updated"

but it looks like updates on this property are not handled in propertyNotifyEvent() implementations in events.cpp
Comment 5 Martin Flöser 2010-08-02 17:29:45 UTC
This could also be a decoration problem. I am pretty sure I do not track 
changes for closable changes in Aurorae.

So just for easier reproducability: which decoration are you using?
Comment 6 Thomas Lübking 2010-08-02 18:49:43 UTC
the decoration can only query isCloseable()* but there's no signal or allowedActionsChanged() function. so kwin needs to call decoration()->reset(SettingButtons).

however I was probably wrong - the allowed_actions are read in getWMHints which is called on _some_ property changes (and many other occasions), so client.cpp:2169

    if( old_allowed_actions == allowed_actions )
        return;
    // TODO: This could be delayed and compressed - It's only for pagers etc. anyway
    info->setAllowedActions( allowed_actions );
+  if (decoration)
+       decoration->reset(KDecoration::SettingButtons);

should do and guess what: there's even a // TODO ;-)

//NOTICE: since my client.cpp is -once more- pulluted with pending review requests i cannot commit a change, sorry... :-P
Comment 7 Andreas Hermann Braml 2010-08-03 18:45:27 UTC
(In reply to comment #5)

I'm using Oxygen decorations (if this piece of information is still needed)
Comment 8 Thomas Lübking 2011-02-06 16:47:29 UTC
Git commit 9a8e4305d574e562c12e9f749322546bc325f014 by Thomas L��bking.
Committed on 06/02/11 at 16:42.
Pushed by luebking into branch 'master'.

Update deco buttons when allowed actions change

BUG: 246442

M  +2    -1    kwin/client.cpp     

http://commits.kde.org/kde-workspace/9a8e4305d574e562c12e9f749322546bc325f014
Comment 9 Andreas Hermann Braml 2012-01-22 13:28:56 UTC
This bug is still present in KDE SC 4.8 RC2
Comment 10 Thomas Lübking 2012-01-22 15:23:35 UTC
Gahh, blast.
The fix notified the decoration whenever kwin changes it's opinion, but that only invokes the motif hint - netwm allowed actions seem to be completely ignored but instead set by kwin - what brings me to the English language and the netwm spec:

If "The Window Manager MUST keep this property updated" reads "must align to it" - this is a kwin bug.
If it reads "must export it's opinion", it's a wine bug which should rely on the window type and the (legacy) motif hints in doubt (and still anyway - whatever is the outcome of this, wine acts wrongly for sure in this regard)

OpenBox acts as kwin, on the other hand compiz (likely following metacity) tracks the property, but that might be an outcome of the implementation (which does not keep internal states at all?)
I'll try E17 later on

The sentence goes on by "... to reflect the actions which are currently "active" or "sensitive" for a window. Taskbars, Pagers, and other tools use _NET_WM_ALLOWED_ACTIONS to decide which actions should be made available to the user."

But i see no suggestion that clients must, should, could, or even might set it.

http://standards.freedesktop.org/wm-spec/wm-spec-latest.html#id2551927
Comment 11 Thomas Lübking 2012-01-22 15:32:48 UTC
PS:
the motif hints change as well. In case that's the "closeability" (it's not like one could read that easily ;-) it should actually update.

What application is this?
Comment 12 Martin Flöser 2012-01-22 15:33:01 UTC
> If "The Window Manager MUST keep this property updated" reads "must align to
> it" - this is a kwin bug.
> If it reads "must export it's opinion", it's a wine bug which should rely on
> the window type and the (legacy) motif hints in doubt (and still anyway -
> whatever is the outcome of this, wine acts wrongly for sure in this regard)
My interpretation of the hint is that the window manager has to set what is 
currently possible and that the client MUST NOT set it. So in my 
interpretation KWin's implementation is correct and wine is wrong.

Did I mention that for Wayland I want a new spec which clearly states what it 
wants to achieve?
Comment 13 Thomas Lübking 2012-01-22 15:56:40 UTC
E17 just ignores the property altogether (doesn't set or track it) - remains the motif hint (which i cannot set via xprop but need some testcase. Ideally w/o reading that stupid header again and writing one but the actually reported one)

And yes: there should be something that deserves the tag "specification" - best unified for Wayland and X11 (ie. backend agnostic, to make client life easier)
Comment 14 Thomas Lübking 2012-01-23 01:41:20 UTC
Created attachment 68104 [details]
testcase

doesn't work in general. the (MWM) hint is updated* but the deco not recreated on the update (but eg. when maximizing or doing anything else impacting abilities)

@Andreas:
can you confirm that eg. maximizing the window shows the close button for you?

*Bespin always shows all buttons but treats them disabled in case.
Comment 15 Thomas Lübking 2012-01-23 01:50:30 UTC
Patch, fixes workspace. My 4.8 is still dirty because of some pending review requests ... ;-)

--- a/kwin/client.cpp
+++ b/kwin/client.cpp
@@ -2003,9 +2003,12 @@ void Client::getMotifHints()
 
     // mminimize; - Ignore, bogus - E.g. shading or sending to another desktop is "minimizing" too
     // mmaximize; - Ignore, bogus - Maximizing is basically just resizing
+    const bool closabilityChanged = motif_may_close != mclose;
     motif_may_close = mclose; // Motif apps like to crash when they set this hint and WM closes them anyway
     if (isManaged())
         updateDecoration(true);   // Check if noborder state has changed
+    if (decoration && closabilityChanged)
+        decoration->reset(KDecoration::SettingButtons);
 }
Comment 16 Thomas Lübking 2012-01-23 01:51:17 UTC
sed -e 's/workspace/testcase/' - it's too late ;-)
Comment 17 Andreas Hermann Braml 2012-01-23 16:49:53 UTC
Weird. I tried again today, and now the "x" is always there (4.8. RC2, no patches). I tried ~30 times, and it always shows!

So, this seems to be fixed after all. If it should _ever_ reoccur, I will revive this reported. Until then: move along, nothing to see here.

But thanks anyway. You even put effort into fixing problems that don't exist :)
Comment 18 Thomas Lübking 2012-01-23 18:55:29 UTC
There's probably a coincident occurrence that updates the decoration (like un/maximizing) or Hugo has changed the oxygen (?) deco to always show the button (ie. act like Bespin/OSX) or wine changed - the testcase is still valid and doesn't work, though you might not be affected (anymore)