phillbush / xfiles

Configurable and simple file manager for X11
MIT License
97 stars 3 forks source link

XEmbedding terminals and editors via xfilesctl is broken #47

Closed rhaberkorn closed 1 week ago

rhaberkorn commented 1 month ago

I am on FreeBSD 14.1-RELEASE-p2.

In examples/xfilesctl you are apparently using XEmbed, for instance in edit().

This is broken for most XEmbed-conforming applications. For instance, simpleterm/st (st -w $WINDOWID) does not work at all - it is not visible. Urxvt (urxvt -embed $WINDOWID) somewhat works, but the window is never focused and does not react to font size changes properly. SciTECO (gsciteco --xembed=$WINDOWID), which is using GtkPlug, does not work at all - nothing is visible while the process is running in the background.

The same btw. is true when using dmenu -w in prompt(), although rofi -dmenu -w $WINDOWID appears to work correctly.

It looks like the Xembed protocol is not fully implemented at the xfiles-side.

rhaberkorn commented 1 month ago

Offtopic: What's that round menu in the screenshot?

rhaberkorn commented 1 month ago

Offtopic: What's that round menu in the screenshot?

That's your pmenu...

phillbush commented 1 month ago

Hi,

There are two issues here:

The first is on how embedded applications create their windows inside the embedder. X11 has this notion of visual types and color depth; and the visual/depth an application uses to draw in their windows must match the visual/depth of the window itself, otherwise a BadMatch error will occur. If the program does not manually set a window's visual/depth, it inherits the ones from its parent window.

In the case of an embedded st(1), its parent window is XFiles. XFiles uses a visual/depth with alpha/opacity channel (so the user can have a semi-transparent background by setting the XFiles.opacity X resource). St however uses X11 default' visual/depth which lacks alpha channel. A BadMatch error then occurs.

I have tested embedded st, and that is the case.
This is what stderr says on a terminal window:

X Error of failed request:  BadMatch (invalid parameter attributes)
  Major opcode of failed request:  70 (X_PolyFillRectangle)
  Serial number of failed request:  419
  Current serial number in output stream:  421

Dmenu also used to have the same problem of BadMatch visual/depth on drawing functions, but it had been fixed by this commit and should work on dmenu 5.3.

I guess other suckless applications will still cause BadMatch.
That is not a bug in XFiles side, although I think I can write a hack for that (maybe map buggy embedded applications into a proxy "dummy" window with default visual/depth, and then reparent then into XFiles actual window).

The second problem is indeed in XFiles side, and that is related to the fact that XFiles does not notify focus changes to the embedded application (which is required by the XEmbed protocol). I am working on a fix for that now.

phillbush commented 1 month ago

Hi,

I sent a patch to st to fix that issue on st side: https://lists.suckless.org/hackers/2408/19158.html

With that patch, st does not inherit embedder's visual information. Rather, it uses X server's default visual information, and no BadMatch error occur while drawing stuff.

I am still working on the issue on XFiles side.

The same btw. is true when using dmenu -w in prompt()

Which version of dmenu are you using? That had been fixed in 5.3.

although rofi -dmenu -w $WINDOWID appears to work correctly

I guess that is because rofi, just like dmenu, is a "focus grabber". They do not depend on the window manager (or embedder) giving them focus. That's why you cannot focus any window while dmenu is active: it has grabbed the focus. Btw, that's the sole reason why I have not implemented XEmbed focus reporting initially: the idea was to embed dmenu only... then i tried to embed xterm and it just works...

rhaberkorn commented 1 month ago

I sent a patch to st to fix that issue on st side: https://lists.suckless.org/hackers/2408/19158.html

Works, cool.

Which version of dmenu are you using? That had been fixed in 5.3.

I tried version 5.1 from the FreeBSD ports tree. So that explains it.

phillbush commented 1 week ago

Hi,

Commit 9eba0d5 implements sending messages to the embedded applications, in particular messageing them when they get focus. I tested with urxvt, which uses the XEMBED_FOCUS_IN message to replace the unfocused "outline-only" cursor box with the filled cursor box.

I shall work fine now.
I will test with Qt and Gtk applications (after I find an embeddable gtk/qt program).

I have not implemented much of the XEmbed protocol, as XFiles does not embed other windows as another widget among others, but as a whole window that replaces XFiles' content, so no complex focus negotiation is required.

rhaberkorn commented 1 week ago

I can confirm that it now works with Urxvt. But SciTECO still fails. :-( The Gtk version uses GtkPlug.

gvim --socketid might also be something to test. It should also be GtkPlug-based.

rhaberkorn commented 1 week ago

And no, gvim --socketid for Gtk3 also doesn't work. Behaves exactly like SciTECO. Both work with tabbed, though.

For some strange reason gvim also doesn't work in my own st fork, where I added Xembed hosting support, while SciTECO does work with it. There is some part of Xembed I also haven't implemented properly yet.

phillbush commented 1 week ago

Hi,

I just tested with gvim and surf (which is also based on gtk) on commit cf2d9f8 and both are embedded fine. I am still having some trouble in making gvim automatically focus its input field. The gvim window is focused, but not the input area.

It seems that gtk does embedding its own way. Even the XEmbed sepecification warns about gtk...

rhaberkorn commented 1 week ago

I just tested with gvim and surf (which is also based on gtk) on commit cf2d9f8 and both are embedded fine.

I suppose you also tested the Gtk3 version of gvim?

phillbush commented 1 week ago

I suppose you also tested the Gtk3 version of gvim?

Yes, the GTK 3 version.

rhaberkorn commented 1 week ago

SciTECO now embeds just fine after your latest changes in cf2d9f8.