Bug 2185 - [PATCH] xprop does not select correct client window in WM's using virtual roots
Summary: [PATCH] xprop does not select correct client window in WM's using virtual roots
Status: CLOSED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: App/xprop (show other bugs)
Version: 6.8.1
Hardware: x86 (IA32) Linux (All)
: high normal
Assignee: James Cloos
QA Contact: Xorg Project Team
URL:
Whiteboard:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2005-01-01 02:55 UTC by Kim Woelders
Modified: 2018-06-17 19:36 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
Fix window selection by pointer (5.02 KB, patch)
2005-01-01 02:59 UTC, Kim Woelders
no flags Details | Splinter Review
Fix window selection by pointer - take 2 (10.29 KB, patch)
2007-08-13 13:19 UTC, Kim Woelders
no flags Details | Splinter Review
xwininfo - Fix window selection by pointer. (10.61 KB, patch)
2007-12-26 01:57 UTC, Kim Woelders
no flags Details | Splinter Review
xwd - Fix window selection by pointer. (10.63 KB, patch)
2007-12-26 01:58 UTC, Kim Woelders
no flags Details | Splinter Review
xwd - Cleanups. (11.31 KB, patch)
2007-12-26 01:58 UTC, Kim Woelders
no flags Details | Splinter Review

Description Kim Woelders 2005-01-01 02:55:39 UTC
When manually selecting a window, xprop finds the top-level window containing
the pointer and uses XmuClientWindow to attempt to find a client window having
WM_STATE set.
In a WM using virtual roots (e.g. enlightenment, desks other than first) this
fails beacuse the top-level window is a virtual root, and XmuClientWindow seems
to find the first client having WM_STATE set, which only by coincidence will be
the one the user intended to select (i.e. containing the pointer).
The solution is to descend the window hierarchy at the pointer location to find
a window with WM_STATE set.

xwd and xwininfo have the same problem, as they use the same function (in
xlsfonts/dsimple.c) to pick the window.

Will attach patch.
Comment 1 Kim Woelders 2005-01-01 02:59:20 UTC
Created attachment 1608 [details] [review]
Fix window selection by pointer
Comment 2 Erik Andren 2006-04-18 05:03:51 UTC
Adding patch keyword
Comment 3 Daniel Stone 2007-02-27 01:24:56 UTC
Sorry about the phenomenal bug spam, guys.  Adding xorg-team@ to the QA contact so bugs don't get lost in future.
Comment 4 Kim Woelders 2007-08-13 13:17:40 UTC
Ok, two and a half year has passed, time to try again... :)

The patch about to be attached should fix various client selection issues such
as the problem with virtual roots (this bug) and the problem with child stack
search order (bug 2135 and bug 7474).

I have put the new window selection code in a separate file so it could be
either copied to the other apps that have the same problems (xwininfo,
xkill, xwd, others?) or included in some library (libXmuu like the broken
XmuClientWindow?).
Comment 5 Kim Woelders 2007-08-13 13:19:08 UTC
Created attachment 11126 [details] [review]
Fix window selection by pointer - take 2
Comment 6 James Cloos 2007-12-06 02:35:52 UTC
I pushed this in commit e09956f244099ddd36b1a2cd5d7800d5fc7120c1.

Patches for other apps like xwd and xwininfo would not be unwelcome....
Comment 7 Kim Woelders 2007-12-26 01:55:23 UTC
First, I should mention that my second patch, which was committed as per comment 6, does not exactly match the description given in comment 1, which was used as the git commit message.

The problem with the original patch was (IIRC) that simply descending the window stack at the pointer location would not find a client if it was selected by clicking its window manager border.

My second patch instead checks if the child of root selected by the mouse click is a virtual root (i.e. listed in _NET_VIRTUAL_ROOTS).
If so the child of the virtual root at the pointer location is used as the starting point (the new subwin) for finding an inferior with WM_STATE set.


The first two patches about to be attached fix the client selection by pointer issue for xwininfo and xwd. The changes are functionally the same as the ones committed to xprop, and the patches duplicate clientwin.c/h code in each of the app directories.
I'm not very happy with duplicating code like this. Maybe it could be moved to some library, as previously suggested? The dsimple.c/h's are also practically identical and could easily be merged.

The third patch performs the same cleanups to xwd/dsimple.c/h that already have been done in the xprop and xwininfo versions.
Comment 8 Kim Woelders 2007-12-26 01:57:11 UTC
Created attachment 13360 [details] [review]
xwininfo - Fix window selection by pointer.
Comment 9 Kim Woelders 2007-12-26 01:58:02 UTC
Created attachment 13361 [details] [review]
xwd - Fix window selection by pointer.
Comment 10 Kim Woelders 2007-12-26 01:58:38 UTC
Created attachment 13362 [details] [review]
xwd - Cleanups.
Comment 11 Kim Woelders 2007-12-26 02:01:05 UTC
Comment on attachment 13361 [details] [review]
xwd - Fix window selection by pointer.

->patch
Comment 12 Kim Woelders 2007-12-26 02:01:58 UTC
Comment on attachment 13362 [details] [review]
xwd - Cleanups.

->patch
Comment 13 Benjamin Close 2008-01-11 02:35:53 UTC
Bugzilla Upgrade Mass Bug Change

NEEDSINFO state was removed in Bugzilla 3.x, reopening any bugs previously listed as NEEDSINFO.

  - benjsc
    fd.o Wrangler
Comment 14 Kim Woelders 2008-06-21 06:05:43 UTC
Ping? Patches made as suggested, about half a year ago.
Comment 15 James Cloos 2008-06-21 22:25:58 UTC
Sorry for the delay.  I let this one slip out of mem.... :^(

Attachment 13360 [details] pushed to xwininfo as c229611bcb7ee94bea5c075f5e15447e14c0f6ce.

Attachment 13361 [details] pushed to xwd as 3dcc66bbbc74c41c2b4509a785c3688fd75387a1.

Attachment 13362 [details] pushed to xwd as f9725928e875035bd2a96621aa8f861160e85dd7.

I’ll also push out tar releases (xwd-1.0.2 and xwininfo-1.0.4).
Comment 16 James Cloos 2008-06-30 06:04:38 UTC
xwd-1.0.2 and xwininfo-1.0.4 tars are pushed.

Closing.

Much thanks!


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.