rust-x-bindings / rust-xcb

Rust bindings and wrapper for XCB.
MIT License
165 stars 64 forks source link

ChangeProperty requests causing bad values for type ATOM_ATOM #144

Closed dangerdyke closed 2 years ago

dangerdyke commented 2 years ago

When I try to change a window property as demonstrated in the examples in the documentation, the resulting window has bizzare, other values for those properties then what they should have been set to. for example, attempting to set _NET_WM_WINDOW_TYPE to ..._DOCK and _NET_WM_STATE to ..._STICKY causes that window to produce the following xprop output:

_NET_WM_DESKTOP(CARDINAL) = 3
WM_STATE(WM_STATE):
        window state: Normal
        icon window: 0x0
WM_PROTOCOLS(ATOM): protocols  audio
_NET_WM_WINDOW_TYPE(ATOM) = ADD_STYLE_NAME
_NET_WM_STATE(ATOM) = 16:9, BT709_YCC
WM_CLASS(STRING) = "bar"

The only property that appears to be setting correctly in this case is WM_CLASS, which was set with the type ATOM_STRING instead of ATOM_ATOM like the others.

my code, for reference: https://gist.github.com/dangerdyke/1a9bd9522567b846ee14dceb10e7f4b8

rtbo commented 2 years ago

Hi, I'm not sure for _NET_WM_WINDOW_TYPE, but for _NET_WM_STATE, the doc states:

[...]
A Client wishing to change the state of a window MUST send a _NET_WM_STATE client message to the root window (see below). 
The Window Manager MUST keep this property updated to reflect the current state of the window.
[...]
To change the state of a mapped window, a Client MUST send a _NET_WM_STATE client message to the root window:

  window  = the respective client window
  message_type = _NET_WM_STATE
  format = 32
  data.l[0] = the action, as listed below
  data.l[1] = first property to alter
  data.l[2] = second property to alter
  data.l[3] = source indication
  other data.l[] elements = 0
This message allows two properties to be changed simultaneously, specifically to allow both horizontal and vertical
maximization to be altered together. l[2] MUST be set to zero if only one property is to be changed.
See the section called “Source indication in requests” for details on the source indication. l[0], the action, MUST be one of:

_NET_WM_STATE_REMOVE        0    /* remove/unset property */
_NET_WM_STATE_ADD           1    /* add/set property */
_NET_WM_STATE_TOGGLE        2    /* toggle property  */

So you need to use x::SendEvent with x::ClientMessageData. You can check here .

yhr0x43 commented 2 years ago

Hi, I just encountered the a very similar problem with _NET_WM_WINDOW_TYPE I can also confirm the specification says it just expect an array of atoms. https://specifications.freedesktop.org/wm-spec/wm-spec-latest.html#idm45381393998896

I did a little digging with strace. I request InternAtom with _NET_WM_WINDOW_TYPE_DOCK. Then request GetAtomName. It reports _NET_WM_WINDOW_TYPE_DOCK as expected. Printing the atom shows it has id 397 on my system (big-endian 32bit [0x8d 0x01 0x00 0x00]) Then a request with ChangeProperty with _NET_WM_WINDOW_TYPE_DOCK as data, the data captured by strace looks like the following: \x12\x00\x07\x00\x00\x00\xc0\x06\x78\x01\x00\x00\x04\x00\x00\x00\x20\x00\x00\x00\x01\x00\x00\x00\0x8d\x00\x00\x00\x2b\x00\x01\x00 The problem is the 24th to 28th byte (\0x8d\x00\x00\x00). It should be the id of _NET_WM_WINDOW_TYPE_DOCK. But it is different by a single bit. I also test with some other program I know also set this same property. They do have the 0x01.

I also tried to request some other atoms to move the id around, but that 1 seems to be missing consistently.

The example code provided by the project also does not properly work for me. This ChangeProperty produce the following result:

NET_WM_DESKTOP(CARDINAL) = 0
WM_STATE(WM_STATE):
        window state: Normal
        icon window: 0x0
WM_PROTOCOLS(ATOM): protocols  RControl
WM_NAME(STRING) = "Basic Window"
yhr0x43 commented 2 years ago

I think I found the problem. In the generated code. sections[4].iov_len = self.data.len() * std::mem::size_of::<u8>(); This particular iov_len should be the size of data in bytes. As an atom is 32 bits or 4 bytes, it should be 4. However, and data.len() just returned the length of array, which is 1, resulted the iov_len to be 1, thus only 1byte out of 4 total is passed to X.

I have yet to look at the generator code to find out how this happens, but I may send a pull request if I figure it out.

rtbo commented 2 years ago

Hi, thanks for tracking this. Can you test if #149 fixes your problem?

yhr0x43 commented 2 years ago

Wow, that is a fast response. I can confirm ChangeProperty with ATOM_ATOM is working properly for me. I do not know how to test the two places in randr that also got changed though.

Also thank you for your work. This really helps me.

rtbo commented 2 years ago

@dangerdyke I'm closing as I think #149 would fix your problem too. If it doesn't, please let me know and I'll reopen.