bug#63511: [Patch] Fix for hidden window bug

bug#63511: [Patch] Fix for hidden window bug

[Christian Schmidt]

[-- Attachment #1: Type: text/plain, Size: 1058 bytes --]

Severity: normal
Tags: Patch

At least on enlightenment, when switching virtual desktop away from the 
on containing emacs and back, emacs does not render screen updates.
This patch fixes the cause.
Analysis for the bug tracker, copied from commit message:

If no state flags are set in the _NET_WM_STATE property,
the returned X property type will be "0". This situation
occurs when "_NET_WM_STATE_HIDDEN" was the only property
set for the emacs window, e.g. in enlightenment due to
change of a virtual desktop.
In the current code this causes copying of the iconified
state in x_get_current_wm_state(), which causes emacs to
stay hidden. For DMs such as Enlightenent or Gnome Shell
that use only the hidden but not the iconified state
this clashes with the intention of handling the
_NET_WM_STATE property notification in
handle_one_xevent(), which requires not_hidden &&
FRAME_ICONIFIED_P (f) to actually activate the frame.

-- 
AIBIoT GmbH        | CEO: Lukas Grunwald
Hornemannstraße 12 | HRB 206588
31137 Hildesheim   | Amtsgericht Hildesheim
Germany

[-- Attachment #2: fix_hidden_window_bug.patch --]
[-- Type: text/x-patch, Size: 1750 bytes --]

commit bed172871fa0a3c1927c25c7664cd9711c303afb
Author: Christian Schmidt <schmidt@digadd.de>
Date:   Sat May 6 13:42:29 2023 +0200

    Fix processing of _NET_WM_STATE for _NET_WM_STATE_HIDDEN
    
    If no state flags are set in the _NET_WM_STATE property,
    the returned X property type will be "0". This situation
    occurs when "_NET_WM_STATE_HIDDEN" was the only property
    set for the emacs window, e.g. in enlightenment due to
    change of a virtual desktop.
    In the current code this causes copying of the iconified
    state in x_get_current_wm_state(), which causes emacs to
    stay hidden. For DMs such as Enlightenent or Gnome Shell
    that use only the hidden but not the iconified state
    this clashes with the intention of handling the
    _NET_WM_STATE property notification in
    handle_one_xevent(), which requires not_hidden &&
    FRAME_ICONIFIED_P (f) to actually activate the frame.
    
    This patch changes the handling of receiving an empty
    property by interpreting it as not hidden.

diff --git a/src/xterm.c b/src/xterm.c
index d621d94a2cf..2bf233b857d 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -27328,7 +27328,7 @@ x_get_current_wm_state (struct frame *f,
                         bool *sticky,
 			bool *shaded)
 {
-  unsigned long actual_size;
+  unsigned long actual_size = 0;
   int i;
   bool is_hidden = false;
   struct x_display_info *dpyinfo = FRAME_DISPLAY_INFO (f);
@@ -27371,9 +27371,8 @@ x_get_current_wm_state (struct frame *f,
       actual_size = actual_bytes / sizeof *reply_data;
       reply_data = xcb_get_property_value (prop);
     }
-  else
+  else if (!prop || prop->type != (Atom) 0)
     {
-      actual_size = 0;
       is_hidden = FRAME_ICONIFIED_P (f);
     }
 #else

bug#63511: [Patch] Fix for hidden window bug

[Stefan Kangas]

Christian Schmidt <cs@aibiot.de> writes:

> At least on enlightenment, when switching virtual desktop away from the
> on containing emacs and back, emacs does not render screen updates.
> This patch fixes the cause.
> Analysis for the bug tracker, copied from commit message:
>
> If no state flags are set in the _NET_WM_STATE property,
> the returned X property type will be "0". This situation
> occurs when "_NET_WM_STATE_HIDDEN" was the only property
> set for the emacs window, e.g. in enlightenment due to
> change of a virtual desktop.
> In the current code this causes copying of the iconified
> state in x_get_current_wm_state(), which causes emacs to
> stay hidden. For DMs such as Enlightenent or Gnome Shell
> that use only the hidden but not the iconified state
> this clashes with the intention of handling the
> _NET_WM_STATE property notification in
> handle_one_xevent(), which requires not_hidden &&
> FRAME_ICONIFIED_P (f) to actually activate the frame.

Po, could you take a look at this patch please.

bug#63511: [Patch] Fix for hidden window bug

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Stefan Kangas <stefankangas@gmail.com> writes:

> Christian Schmidt <cs@aibiot.de> writes:
>
>> At least on enlightenment, when switching virtual desktop away from the
>> on containing emacs and back, emacs does not render screen updates.
>> This patch fixes the cause.
>> Analysis for the bug tracker, copied from commit message:
>>
>> If no state flags are set in the _NET_WM_STATE property,
>> the returned X property type will be "0". This situation
>> occurs when "_NET_WM_STATE_HIDDEN" was the only property
>> set for the emacs window, e.g. in enlightenment due to
>> change of a virtual desktop.

Does Enlightenment _remove_ the property, or merely set its type to
something other than ATOM?

> Po, could you take a look at this patch please.

That's a bug in Enlightenment, as the type of _NET_WM_STATE should
always be set to an array of atoms irrespective of its contents.

If the window manager outright removes the property, clients have no
means to ascertain if the window manager has relinquished control over
the window, or it is still iconified by some other means (such as the
traditional WM_STATE).

I would prefer a check against that other property when that is the
case, rather than overriding the results of the loop below.

bug#63511: [Patch] Fix for hidden window bug

[Christian Schmidt]

On 8/24/23 02:12, Po Lu wrote:
> Stefan Kangas <stefankangas@gmail.com> writes:
> 
>> Christian Schmidt <cs@aibiot.de> writes:
>>
>>> At least on enlightenment, when switching virtual desktop away from the
>>> on containing emacs and back, emacs does not render screen updates.
>>> This patch fixes the cause.
>>> Analysis for the bug tracker, copied from commit message:
>>>
>>> If no state flags are set in the _NET_WM_STATE property,
>>> the returned X property type will be "0". This situation
>>> occurs when "_NET_WM_STATE_HIDDEN" was the only property
>>> set for the emacs window, e.g. in enlightenment due to
>>> change of a virtual desktop.
> 
> Does Enlightenment _remove_ the property, or merely set its type to
> something other than ATOM?

prop->type == (Atom) 0 IMHO refers to an empty list, which is valid.

>> Po, could you take a look at this patch please.
> 
> That's a bug in Enlightenment, as the type of _NET_WM_STATE should
> always be set to an array of atoms irrespective of its contents.

I would be surprised if this was a bug in E, given emacs is the only 
program I know of that exhibits this issue. I have discussed this issue 
extensively with raster, the main author of E, and we have come to the 
same conclusion: when _NET_WM_STATE_HIDDEN  the is the only active/set 
property, and then is removed/unset (sorry if my terminology is wrong, 
my days of X programming have been 20+ years ago, and these are corners 
I have never touched before), there is no code path in emacs that can 
handle this case. Also, the request does not hit the window manager, but 
it is X that replies. As such, the bug would lie within X11.

What happens is that the array of atoms is empty, and thus the list is 
(Atom) 0. Currently this is interpreted as "keep the current state", and 
not to re-evaluate the (now empty) list of atoms to realize that 
_NET_WM_STATE_HIDDEN is not set.

The scenario in which this occurs is a switch of a virtual desktop in E. 
E will only hide affected windows, not iconify them, which seems 
correct. E uses compositing and updates even windows on other virtual 
desktops in the pager when (and if) they redraw while hidden.

> If the window manager outright removes the property, clients have no
> means to ascertain if the window manager has relinquished control over
> the window, or it is still iconified by some other means (such as the
> traditional WM_STATE).

If that would be an issue, given there is no "unhidden" property that 
could be set, how should the WM handle this? E sets _NET_WM_STATE_HIDDEN 
when the window is hidden, and removes it when no longer hidden.

> I would prefer a check against that other property when that is the
> case, rather than overriding the results of the loop below.

In summary: Two scenarios currently can cause SET_FRAME_ICONIFIED (f, 
true): _NET_WM_STATE_HIDDEN and wm_state in Iconic_State. A removal of a 
final property _NET_WM_STATE_HIDDEN does not trigger SET_FRAME_ICONIFIED 
(f, false), because of what appears to me like a misinterpretation of an 
empty Atom list.

However, as long as any other property, e.g. sticky, would be set, 
removing _NET_WM_STATE_HIDDEN would behave like the patched version.

I would agree that the check could be reasonable, though I can't really 
imagine a situation in which iconified+hidden is set, yet only hidden 
would be removed. In fact, the implementation notes give iconified as 
the typical use case of hidden, so these would go hand in hand.

This patch only adds correct handling of an empty _NET_WM_STATE Atom 
list, and does not otherwise alter behavior as described above. The 
unconditional initialization of actual_size seems ok to me as the 
alternative would be to have it set twice, once in the else tree, and 
secondly in another new else tree. I'd say both from code size and 
runtime performance this should be the better option.

Best regards,
Christian

bug#63511: [Patch] Fix for hidden window bug

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Christian Schmidt <cs@aibiot.de> writes:

> prop->type == (Atom) 0 IMHO refers to an empty list, which is valid.

I'm sorry, but that contradicts the EWMH, which prescribes _NET_WM_STATE
as an array of Atom.  prop->type is only None (which is _not_ a type)
when the property is deleted.

> I would be surprised if this was a bug in E, given emacs is the only
> program I know of that exhibits this issue. I have discussed this
> issue extensively with raster, the main author of E, and we have come
> to the same conclusion: when _NET_WM_STATE_HIDDEN  the is the only
> active/set property, and then is removed/unset (sorry if my

_NET_WM_STATE_HIDDEN is not a property that may be ``set'' or ``unset''
(mind that this parlance is not X terminology), it's an element in a
property named _NET_WM_STATE.  Each window property is in fact an array
of items whose width in bits is the format of the property.

That property becomes empty when _NET_WM_STATE_HIDDEN is the only
element of that array and is subsequently removed; when that transpires,
GetWindowProperty still returns an empty array of the correct type, as
the property is still set.

The only two situations in which GetWindowProperty will return a type of
None is when Enlightenment sets the property type to None or deletes the
property in its entirety, both of which violate the EWMH.

> terminology is wrong, my days of X programming have been 20+ years
> ago, and these are corners I have never touched before), there is no
> code path in emacs that can handle this case. Also, the request does
> not hit the window manager, but it is X that replies. As such, the bug
> would lie within X11.

When a window property is changed to an empty value, its type remains
intact.  Enlightenment is not merely changing the property in this case,
but also deleting it, which contravenes the EWMH.

> What happens is that the array of atoms is empty, and thus the list is
> (Atom) 0. Currently this is interpreted as "keep the current state",
> and not to re-evaluate the (now empty) list of atoms to realize that
> _NET_WM_STATE_HIDDEN is not set.

An empty window property is empty -- its type remains intact, regardless
of the number of items within.  There's a distinct contrast between an
empty window property and a window property that has abruptly vanished.

> If that would be an issue, given there is no "unhidden" property that
> could be set, how should the WM handle this? E sets
> _NET_WM_STATE_HIDDEN when the window is hidden, and removes it when no
> longer hidden.

The window manager should empty the property instead of deleting it,
which is more or less proscribed by convention.  The removal of a WM
property from a client toplevel window should always coincide with a
corresponding change to _NET_SUPPORTED on the root window.