Fix menu dropdown focus logic in xwayland

[salkin-mada]

Not sure what unforeseen results this will have. But it seems to close #6324.
I tested with code (vscode) running under xwayland and the menu's seemed to work fine there without setting seat focus to NULL before setting to previous.

sway/sway/desktop/xwayland.c

Line 118 in bb3fd0a

seat_set_focus(seat, NULL);

At least removing that line makes the drop down menu's in Reaper usable again.

[kennylevinsen]

7479993

[salkin-mada]

So maybe I should get that code back into
void unmanaged_handle_unmap.
Instead of just removing the NULL set.
Will test. At least as far as I can tell the nice line seat_set_focus(seat, previous->parent); from 7479993 is no longer present in the master . It has been changed to seat_set_focus(seat, NULL);

[salkin-mada]

Okay. Kinda skip my last comment :)~. @yrhki was right. Adding back 7ca9ef1 works in regards to the inherent / immediate bug report in #6324.
Will need to test some more. Floating window [FX windows] seems still to have the frozen drop down bug.

[salkin-mada]

It works smoothly here on my subproject compiled version.
Thanks to @yrhki . Apparently I don't understand how to add co-authors via command line, plain old git.

[yrhki]

@salkin-mada can you add my email contact@yrhki.com.

[salkin-mada]

Nice @yrhki. I tried to grap that via the github api. But no luck.
Will amend.

[kennylevinsen]

Taking a glance at the git history alone, it needs some cleaning. You have one commit doing your first fix attempt, and another undoing that and doing your second fix attempt. The message on the second one is also a little weird, including ">" characters it seems?

See https://chris.beams.io/posts/git-commit/ for examples. For where to put "Co-authored-by", look for the examples containing a "See also" - that's the "block" you'd use for that.

It would be good if the commit summary and description explained what is being added, and then in the description mention that this comes from that other commit. Most people do not remember all the sha's in the tree, so "re-add commit xyz" doesn't mean a lot to most.

[salkin-mada]

thanks @kennylevinsen . Will do.

[Xyene]

I have mixed feelings about this patch. I suspect we're missing something fundamental, because other compositors do not have such complex X11 refocusing logic, and presumably (has anyone tried Weston?) Reaper works there. The last focus issues we had with JetBrains IDEs ended up being due to us not implementing ICCCM focus hints; I wonder if that is at play here.

This "try to find another unmanaged surface from the same process to pass focus to" logic is a hack I initially wrote without understanding why it was necessary, so I'd be a little sad to see it get reintroduced. It may also be informative to trace through the code and understand why removing seat_set_focus(seat, NULL); is necessary in Sway, and why removing it fixes things in Reaper.

/cc @BrassyPanache

[salkin-mada]

@Xyene Nice to see you joining in. We have been loosely throwing around the "lets find a universal fix" talk.
And Indeed. Making a more condense and real fix (understanding the inner workings of these focus issues/symptoms) seems way more intriguing than just adding back the "try to find another unmanaged surface from the same process to pass focus to" attitude.
One thing to note about the seat_set_focus(seat, NULL); and Reaper is that it fixes the immediate loosing focus of drop down menu bug in Reaper. But adding back 7ca9ef1 fixes the issue way deeper. One example is using the ESC key while having several drop downs open/unfolded. It will close them correctly in order all the way "down" to the main menu.
So just removing seat_set_focus(seat, NULL); does not really really do the same.

[BrassyPanache]

I can share how I approached the issue swaywm/wlroots#2604 (related pulls of swaywm/wlroots#2605 and #5948). The issue sounds very similar in nature - IntelliJ drop down menus were behaving incorrectly.

The initial fix I put in place was for #3007 where I took a seemingly similar approach to what's taken here. To make the fix I inserted logging statements until I could work out the specific change required to get IntelliJ to work and then made the change. I then verified that other applications worked as expected and called it a day.

Of course this change broke with the next IntelliJ release. I was not at the time privy to the ICCCM specifications regarding input focus. I found the following table most insightful.

I would just check that everything is consistent to the above table from both the sway (and wlroots) side and the Reaper side. If other window managers (e.g. GNOME) work it is a good indication that the change needs to occur in sway or wlroots.

I am by no means an expert on these matters - just sharing my prior experience 😄

[madskjeldgaard]

Any progress on this? It is still a critical issue and it would be nice to have it fixed soon. Thanks!

[emersion]

"Me too", "just merge it", "I won't use Sway unless this is fixed" type of comments don't help. If you want to help, read the discussion above and find a proper fix. A workaround like the one done currently in this PR won't get merged.

[elmarsto]

Cool, cool. I wish you all the best. Thank you for sway! It will be sorely missed. So long for now.
✨

[salkin-mada]

Fixed in #6764 🎉

[martin-greentrax]

Hey everyone,
really appreciate your great work with swaywm, I found this issue after spending a long time trying to get popups in a java/xwayland app TWS of interactive brokers to not vanish and found it very similar. Could you kindly check out my bug report here: #7823

Thank you :)