fix: persistance for client.ontop across restarts (via X properties)

[ViStefan]

I've noticed that client.ontop state is not persistent across awesome restarts. (It also isn't persistent after going fullscreen, but x layers somehow requires that, according to sourcecode.)

Ater a minute of research, I've found that X11 WM protocol doesn't have an atom for ontop state at all by convention. There are only layers like DOCK, ABOVE, etc.

I've added _NET_WM_STATE_ONTOP atom. This fixed that for me. Is it legal?

[codecov[bot]]

Codecov Report

Merging #3389 (b6a5cc5) into master (8541637) will decrease coverage by 0.42%. The diff coverage is 30.00%.

:exclamation: Current head b6a5cc5 differs from pull request most recent head a6dab33. Consider uploading reports for the commit a6dab33 to get more accurate results

@@            Coverage Diff             @@
##           master    #3389      +/-   ##
==========================================
- Coverage   90.17%   89.74%   -0.43%     
==========================================
  Files         821      739      -82     
  Lines       53295    49168    -4127     
==========================================
- Hits        48057    44126    -3931     
+ Misses       5238     5042     -196     

Flag	Coverage Δ
gcov	75.24% <30.00%> (-14.93%)	:arrow_down:
luacov	92.60% <ø> (-0.67%)	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
ewmh.c	62.77% <30.00%> (-1.55%)	:arrow_down:
lib/awful/client/focus.lua	56.47% <0.00%> (-35.39%)	:arrow_down:
lib/awful/client/urgent.lua	63.15% <0.00%> (-26.32%)	:arrow_down:
lib/awful/client.lua	55.63% <0.00%> (-22.34%)	:arrow_down:
lib/awful/screen.lua	84.48% <0.00%> (-8.48%)	:arrow_down:
lib/awful/wibar.lua	80.79% <0.00%> (-7.95%)	:arrow_down:
lib/wibox/widget/imagebox.lua	90.12% <0.00%> (-5.42%)	:arrow_down:
lib/awful/_compat.lua	75.60% <0.00%> (-3.12%)	:arrow_down:
common/atoms.c	88.88% <0.00%> (-2.78%)	:arrow_down:
objects/selection_transfer.c	94.18% <0.00%> (-2.41%)	:arrow_down:
... and 192 more

[ViStefan]

It is not too much about real merging, but I just want to discuss, is it a good idea at all, or it breaks conventions? If it's all right, I'll cover it with tests.

Anybody?

[Aire-One]

Sorry for the delay. We are not very active latterly, and it's summer now, so I guess everyone is busy at this time...

AFAIK, we try to not add new atoms that are not standard and that would be only interpreted by us, in awesome. I'm not very familiar with this subject, so maybe there is a world where this ONTOP atom is used by other, and we should also support it?

@psychon, @Elv13 what's your point of view of the situation?

I've noticed that client.ontop state is not persistent across awesome restarts.

About persistence across awesome restarts, there is not too much we can do. The recommended way to persist data would be to write them somewhere on disk and read this file at awesome start, in the end user rc.lua to apply back the properties to your clients. I don't remember having seen any real implementation of this before, tho...

It also isn't persistent after going fullscreen, but x layers somehow requires that, according to sourcecode.

I don't know about the X.org requirement, but there is probably something we could do at our level, in awesome, to persist the data when a client goes fullscreen. (I'm actually surprised we lost the ontop property value when fullscreen is altered...)

[ViStefan]

Im actually surprised we lost the ontop property value when fullscreen is altered

There's no wodner. If window has both ontop and fullscreen, other windows will be unusable, because whole screen is obtained by ontop window. :)

https://github.com/awesomeWM/awesome/blob/906dc543e423bac439942e4c755de1e452b9e1ff/objects/client.c#L2550-L2557

Probably it would be better to restore ontop status after leaving fullscreen, but it is possible to be made with rc.lua.

[Aire-One]

Im actually surprised we lost the ontop property value when fullscreen is altered

There's no wodner. If window has both ontop and fullscreen, other windows will be unusable, because whole screen is obtained by ontop window. :)

It makes a lot of sense. Thanks for this!

Probably it would be better to restore ontop status after leaving fullscreen, but it is possible to be made with rc.lua.

I don't know how we could implement this feature correctly. Also, I'm afraid it is the kind of breaking changes we don't want to introduce in awesome...

Hm... Yeah, I think it is doable from the end user config file. It should be easy enough to create a list of ontop clients and check it when a client exists fullscreen. (Even easier: create a custom property directly attached to the client object itself)

[psychon]

Well... there is no _NET_WM_STATE_ONTOP in https://specifications.freedesktop.org/wm-spec/wm-spec-1.3.html, so... yeah. However, I don't see any reason why this couldn't be persisted in a custom property. We already do that with floating in lib/awful/client.lua:

-- Register persistent properties
client.property.persist("floating", "boolean")

However, I guess ontop cannot be as easily persisted from Lua since the property itself is implemented in C. Quick untested attempt that doesn't touch any C code:

local xprop = "awful.client.property.ontop"
awesome.register_xproperty(xprop, "boolean")
client.connect_signal("manage", function(c)
    local value = c:get_xproperty(xprop)
    if value ~= nil then
        c.ontop = value
    end
end)
client.connect_signal("property::ontop", function(c)
    c:set_xproperty(xprop, c.ontop)
end)

The above is completely untested, but dumping that into lib/awful/client.lua could be easier than implementing the same thing in C.

[ViStefan]

The above is completely untested, but dumping that into lib/awful/client.lua could be easier than implementing the same thing in C.

I'll try this and update request.