xpra icon
Bug tracker and wiki

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#138 closed defect (fixed)

"git gui": click only after moving window

Reported by: pmarek Owned by: Antoine Martin
Priority: minor Milestone: 0.3
Component: client Version: 0.3.x
Keywords: Cc:

Description

When I start "git gui", I get the initial window asking which GIT checkout I want to use.

I cannot click on the path already shown there; if I move the window, I can click.

Reproduceable by ahuillet.

Attachments (6)

git-gui-newpath.png (73.5 KB) - added by Antoine Martin 7 years ago.
starting "git gui" in a path which does not have a repository
git-gui-existingpath.png (158.0 KB) - added by Antoine Martin 7 years ago.
starting "git gui" in a path which has an existing repository
fluxbox-start-git-gui-new-path.png (27.2 KB) - added by Antoine Martin 7 years ago.
starting "git gui" in a path which does not have a repository (with fluxbox desktop)
fluxbox-git-gui-can-select-recent-project.png (65.6 KB) - added by Antoine Martin 7 years ago.
once started, I can select an existing project immediately (fluxbox desktop)
fluxbox-start-git-gui-existing-path.png (25.8 KB) - added by Antoine Martin 7 years ago.
starting "git gui" in a path which has an existing repository (fluxbox desktop)
fluxbox-git-gui-existing-path-works.png (191.8 KB) - added by Antoine Martin 7 years ago.
"git gui" project window is responsive as soon as it is started (fluxbox desktop)

Download all attachments as: .zip

Change History (21)

comment:1 Changed 7 years ago by Antoine Martin

Status: newaccepted

Probably window focus related, moving the window ensures it is focused, unfocusing it (from the local window manager's point of view) before clicking in that area probably does the same thing.

Focus is a pain, you have X11 focus, xpra focus, client-side focus... getting them all to agree in an asynchronous world is difficult at best.

Please try various client-side window managers, and at least tell me the one you have which did not work. I'm pretty sure some do work ok (there were similar problems before - see #51 r357 r356 r231).

git gui is written in Tcl/Tk, it may be worth trying other Tcl/Tk applications to see if they exhibit the same problem. Focus is generally handled by the toolkit, not by the app directly.

comment:2 Changed 7 years ago by pmarek

This is with debian testing/unstable, KDE 4.7.4.

comment:3 Changed 7 years ago by ahuillet

Fluxbox exhibits the problem but not where the OP described it - clicking the path works, but the window that appears *then* is unresponsive until I move it.

Changed 7 years ago by Antoine Martin

Attachment: git-gui-newpath.png added

starting "git gui" in a path which does not have a repository

Changed 7 years ago by Antoine Martin

Attachment: git-gui-existingpath.png added

starting "git gui" in a path which has an existing repository

comment:4 Changed 7 years ago by Antoine Martin

As per the screenshots above, I cannot see the "path already shown there" that you refer to.

Please provide more detailed steps on how to reproduce this issue.

comment:5 Changed 7 years ago by ahuillet

The window in your "existingpath" screenshot is the one that is unresponsive in my case.
The window in your "newpath" screenshot - is the one that is unresponsive for the OP, *but* in the case where there is a path displayed there.

comment:6 Changed 7 years ago by Antoine Martin

OK, and any idea how would I get a path to display there?

FWIW: both work just fine for me. LXDE on Fedora 16.

comment:7 Changed 7 years ago by ahuillet

Open git-gui in a directory where there is no repository. You will get your "newpath" image. Click open, select a path. You'll get the "existing path" image.
Quit git-gui. Start it again in the same directory where there is no repository. This time you should see the path appear (as a "last used" path).

Changed 7 years ago by Antoine Martin

starting "git gui" in a path which does not have a repository (with fluxbox desktop)

Changed 7 years ago by Antoine Martin

once started, I can select an existing project immediately (fluxbox desktop)

Changed 7 years ago by Antoine Martin

starting "git gui" in a path which has an existing repository (fluxbox desktop)

Changed 7 years ago by Antoine Martin

"git gui" project window is responsive as soon as it is started (fluxbox desktop)

comment:8 Changed 7 years ago by Antoine Martin

Finally managed to reproduce the KDE one (was doing it wrong in the KDE screenshots - edit: now removed them). Looks like a KDE bug to me, maybe related to that recent click vs drag thing with KDE windows.
I have kde 4.8.3, will try to find out more about what is going on here, but I am not certain this can be solved in xpra.

I simply cannot reproduce the fluxbox one however, no matter how hard I try.
It goes to the project shown as soon as the mouse is clicked (button-down, it does not even wait for the corresponding button-up)
Fluxbox 1.3.2

I tested this by starting a new fluxbox/KDE "desktop session" with VNC via winswitch (you can also do it by hand if you feel so inclined). Since the focus never leaves the VNC window, I doubt a "real" DISPLAY=:0 session will make much difference, but I will try anyway.

Last edited 7 years ago by Antoine Martin (previous) (diff)

comment:9 Changed 7 years ago by Antoine Martin

Simply by adding this:

Index: src/xpra/client.py
===================================================================
--- src/xpra/client.py	(revision 884)
+++ src/xpra/client.py	(working copy)
@@ -416,6 +416,7 @@
                                           pointer, modifiers])
 
     def _button_action(self, button, event, depressed):
+        log.info("button_action(%s,%s,%s)" % (button, event, depressed))
         if self._client.readonly:
             return
         (pointer, modifiers) = self._pointer_modifiers(event)

I can see that the clicks are being passed from the KDE client to the xpra server. Why the window/toolkit decide to ignore them... is another matter.

comment:10 Changed 7 years ago by Antoine Martin

Also reproducible with the win32 client.

comment:11 Changed 7 years ago by Antoine Martin

OK, here is a log of everything related to focus calls:

  • on the server:
    _process_focus(['focus', 5, []])
    _focus(5,[]) has_focus=0
    _focus(5,[]).give_focus()
    Giving focus to client
    ... using XSetInputFocus
    
  • on the client:
    _focus_change(()) _been_mapped=True
    _focus_change((<ClientWindow object at 0x23a3370 (xpra+client+ClientWindow at 0x22e5350)>,
                   <GParamBoolean 'has-toplevel-focus'>)) _been_mapped=True
    send_focus(5)
    

The client window's _focus_change fires and we tell the server which then uses XSetInputFocus to give it focus (to mimic what the client has already done).

In the case where it does not work, the calls are identical.. race?

First note: we fire first from do_map_event via idle_add then via the notify::has-toplevel-focus signal (sometimes the other way around too?).
Removing the first call does not change anything unfortunately, and I seem to remember that this was necessary for some window managers which fail to fire the "has-toplevel-focus" signal despite actually giving focus.. (unity irrc - might be worth checking if this is fixed to remove the double call)

Not sure this is relevant, but this sequence of events does happen when we click on the "git gui" window widget to do both focus and click in one go:

_process_focus(['focus', 6, []])
_focus(6,[]) has_focus=0
_process_button_action(['button-action', 6, 1, 1, [197, 166], []])
_focus(6,[]).give_focus()
Giving focus to client
... using XSetInputFocus
_process_button_action(['button-action', 6, 1, 0, [197, 166], []])

The focus happens between the mouse-down and mouse-up.

We had to add gobject.idle_add to fix focus problems on win32... And now we get another one?! See r231 and the docstring it adds:

#no idea why we can't call this straight away!
#but with win32 clients, it would often fail!???
Last edited 7 years ago by Antoine Martin (previous) (diff)

comment:12 Changed 7 years ago by Antoine Martin

I don't understand why this makes any difference, something asynchronous in X11 is making the "move-window" re-do what we already do in configure_window and that makes everything happy... but why? (this will be the next step)

In the meantime, here's a workaround, please let me know if that fixes the issue in *ALL* cases for you:

Index: src/xpra/client.py
===================================================================
--- src/xpra/client.py	(revision 884)
+++ src/xpra/client.py	(working copy)
@@ -359,7 +359,9 @@
                 ox, oy = self._pos
                 dx, dy = x-ox, y-oy
                 self._pos = (x, y)
-                self._client.send(["move-window", self._id, x, y])
+            self._client.send(["move-window", self._id, x, y])
+            if (x, y) != self._pos:
+                log.info("window moved to %sx%s", x, y)
                 for window in self._override_redirect_windows:
                     x, y = window.get_position()
                     window.move(x+dx, y+dy)

comment:13 Changed 7 years ago by Antoine Martin

fixed in r905, feel free to re-open if you find an app or toolkit that still misbehaves...

Tested with KDE and win32. Will apply to 0.3.x branch after further testing.

comment:14 Changed 7 years ago by Antoine Martin

Resolution: fixed
Status: acceptedclosed

applied to 0.3.x branch in r951

comment:15 Changed 7 years ago by Antoine Martin

I suspect that the more correct fix for this might well be r2003 (backported as r2004) - someone with spare time could confirm.

Note: See TracTickets for help on using tickets.