On Sat, May 17, 2008 at 03:12:49AM -0700, Klaus Weidner wrote:

Hello,

thanks David for taking care of this - I'm currently away from my xmonad system and can't try to reproduce it, but your change should fix the loop.

Is something like the following happening?

a) click on tab causes window B to be focused (with mouseFocused true)

b) mouse moves down into old window A, queueing an enter event for A

c) xmonad maps window B, appearing under the mouse pointer

d) enter event for B gets queued

e) xmonad handles enter event for A, causing it to be focused

f) xmonad maps window A, appearing under the mouse pointer

g) enter event for B gets queued

... goto (c)

I suspect this is what happens.

The problem with my patch would then be that it keeps mouseFocused on for all these enter events for passing to updatePointer and also uses that same flag to decide not to clear the pending crossing events, which is wrong.

The unpatched behaviour restored by your patch is to clear the step (b) enter event. However, that does the wrong thing if moving the mouse quickly across multiple windows A-B-C, in that case B keeps the focus even though the mouse ended in window C.

I've never observed this, myself. Perhaps it requires an even slower computer than mine, or maybe a really slow layout?

Probably the right thing to do would be to clear the pending crossing events in 'windows' whenever the layout manipulation moves/resizes/remaps windows, and not clear it if only the focus has changed. What would be a clean way to make that distinction?

Probably a better option would be to not clear crossing events at all, but instead when we handle crossing events to first check if the mouse is still in the said window. If it's not, then we should ignore the event.

-Klaus

On Fri, May 16, 2008 at 04:13:57PM -0400, David Roundy wrote:

...

Here's a patch to core to fix a recent regression in xmonad. Well, maybe it's not so recent, but I've only recently upgraded my xmonad to the latest darcs version. It's a crude fix that leaves in place the whole isMouseFocused infrastructure and just disables its effect. Without this patch, tabbed is not entirely unuseable, but requires that I train myself to hold the mouse very still while selecting tabs. I'd rather just have the old pleasant behavior where I can select even firefox windows without waiting before letting my mouse drift and without annoying flashing effects.

David

Fri May 16 15:09:47 CDT 2008 David Roundy * quick hack to avoid infinite (but breakable) loop when using Tabbed.

The isMouseFocused hack introduced a race condition where if you click on a tab to select a window, and then move the mouse down before the new window is selected, the focus will alternate between the previously-focused and newly-focused windows in an infinite loop. This loop can be broken by moving the mouse up to the tab area, but it's pretty unpleasant. This race condition is easily triggered on my laptop (and presumably on any older computer running bloated applications like firefox).

I've disabled the responsible behavior in a minimal way, since it seems likely that someone may wish to reimplement this behavior, but without the race condition.

Content-Description: A darcs patch for your repository!

...

New patches:

[quick hack to avoid infinite (but breakable) loop when using Tabbed. David Roundy **20080516200947

The isMouseFocused hack introduced a race condition where if you click on a tab to select a window, and then move the mouse down before the new window is selected, the focus will alternate between the previously-focused and newly-focused windows in an infinite loop. This loop can be broken by moving the mouse up to the tab area, but it's pretty unpleasant. This race condition is easily triggered on my laptop (and presumably on any older computer running bloated applications like firefox).

I've disabled the responsible behavior in a minimal way, since it seems likely that someone may wish to reimplement this behavior, but without the race condition. ] hunk ./XMonad/Operations.hs 169 mapM_ (flip setWMState withdrawnState) (W.allWindows old \\ W.allWindows ws)

isMouseFocused <- asks mouseFocused - unless isMouseFocused $ clearEvents enterWindowMask + unless (isMouseFocused && False) $ clearEvents enterWindowMask

-- | setWMState. set the WM_STATE property setWMState :: Window -> Int -> X ()

Context:

[add currentTag convenience function Devin Mullins **20080511224258] [Make Mirror a newtype Spencer Janssen **20080508104640] [Comments Spencer Janssen **20080507013122] [Break long line Spencer Janssen **20080507012608] [Style Spencer Janssen **20080507012519] [Simplify Spencer Janssen **20080507011309] [Overhaul Choose, fixes issue 183 Spencer Janssen **20080506220809] [Remember if focus changes were caused by mouse actions or by key commands Klaus Weidner **20080502175603

If the user used the mouse to change window focus (moving into or clicking on a window), this should be handled differently than focus changes due to keyboard commands. Specifically, it's inappropriate to discard window enter/leave events while the mouse is moving. This fixes the bug where a fast mouse motion across multiple windows resulted in the wrong window keeping focus.

It's also helpful information for contrib modules such as UpdatePointer - it's supposed to move the mouse pointer only in response to keyboard actions, not if the user was moving the mouse. ] [Wibble Spencer Janssen **20080506203840] [Added doShift function for more user-friendly hooks Ivan N. Veselov **20080506185757] [use named colours. fixes startup failure on the XO Don Stewart **20080502210149] [Set focus *after* revealing windows Spencer Janssen **20080407222559] [Reveal windows after moving/resizing them. Spencer Janssen **20080407220756 This should reduce the number of repaints for newly visible windows. ] [Hide newly created but non-visible windows (fixes bug #172) Spencer Janssen **20080430014012] [formatting, eta expansion Don Stewart **20080418184337] [XMonad.ManageHook: add 'appName', another name for 'resource' Lukas Mai **20080406012006] [XMonad.ManageHook: make 'title' locale-aware; haddock cleanup Lukas Mai **20080406011338

The code for 'title' was stolen from getname.patch (bug #44). ] [XMonad.Main: call setlocale on startup Lukas Mai **20080406011234] [floats always use current screen (with less bugs) robreim@bobturf.org**20080405135009] [XMonad.Operations: applySizeHint reshuffle Lukas Mai **20080404215615

Make applySizeHints take window borders into account. Move old functionality to applySizeHintsContents. Add new mkAdjust function that generates a custom autohinter for a window. ] [XMonad.Layout: documentation cleanup Lukas Mai **20080404215444] [Remove gaps from the example config Spencer Janssen **20080329232959] [Remove gaps Spencer Janssen **20080325091526] [TAG 0.7 Spencer Janssen **20080329210249] Patch bundle hash: f3efdbe61ee5bcdbb219553c7229617caae51b17

...

_______________________________________________ xmonad mailing list xmonad@haskell.org http://www.haskell.org/mailman/listinfo/xmonad

-- David Roundy Department of Physics Oregon State University

On Sat, May 17, 2008 at 11:18:53AM -0700, Klaus Weidner wrote:

On Sat, May 17, 2008 at 05:09:41AM -0700, David Roundy wrote:

...

On Sat, May 17, 2008 at 03:12:49AM -0700, Klaus Weidner wrote:

...

The unpatched behaviour restored by your patch is to clear the step (b) enter event. However, that does the wrong thing if moving the mouse quickly across multiple windows A-B-C, in that case B keeps the focus even though the mouse ended in window C.

I've never observed this, myself. Perhaps it requires an even slower computer than mine, or maybe a really slow layout?

The notebook I saw this on is pretty fast (Core 2 Duo), and I commonly saw this with a moderately complex (but not unreasonable) dualscreen setup including combineTwo and tabs on one screen, something like this:

Ah, I see, this involves two screens. That explains why I never saw it.

...

...

Probably the right thing to do would be to clear the pending crossing events in 'windows' whenever the layout manipulation moves/resizes/remaps windows, and not clear it if only the focus has changed. What would be a clean way to make that distinction?

Probably a better option would be to not clear crossing events at all, but instead when we handle crossing events to first check if the mouse is still in the said window. If it's not, then we should ignore the event.

That sounds reasonable - how about a something like the following (not a real patch - I'm still away from my computer including next week and can't test this):

XMonad/Main.hs: handle e@(CrossingEvent {ev_window = w, ev_event_type = t}) | t == enterNotify && ev_mode e == notifyNormal && ev_detail e /= notifyInferior - = whenX (asks $ focusFollowsMouse . config) (focus w) + = whenX (asks $ focusFollowsMouse . config) $ do + dpy <- asks display + root <- asks theRoot + (_,_,w',_,_,_,_,_) <- io $ queryPointer dpy root + when (w == w') (focus w)

XMonad/Operations.hs: mapM_ (flip setWMState withdrawnState) (W.allWindows old \\ W.allWindows ws) - - unless isMouseFocused $ clearEvents enterWindowMask

Looks reasonable to me. Of course, we'd probably also want to remove isMouseFocused entirely, right? David

I've tried the change described below (see attached patch), and it works for me, and appears to also make xmonad react a lot faster to mouse movement due to skipping unnecessary refocusing. Can others please check if it causes any regressions for them? -Klaus On Sat, May 17, 2008 at 11:18 AM, Klaus Weidner wrote:

On Sat, May 17, 2008 at 05:09:41AM -0700, David Roundy wrote:

...

On Sat, May 17, 2008 at 03:12:49AM -0700, Klaus Weidner wrote:

...

The unpatched behaviour restored by your patch is to clear the step (b) enter event. However, that does the wrong thing if moving the mouse quickly across multiple windows A-B-C, in that case B keeps the focus even though the mouse ended in window C.

I've never observed this, myself. Perhaps it requires an even slower computer than mine, or maybe a really slow layout?

The notebook I saw this on is pretty fast (Core 2 Duo), and I commonly saw this with a moderately complex (but not unreasonable) dualscreen setup including combineTwo and tabs on one screen, something like this:

Screen A Screen B -- firefox windows -- -- other - [tab][tab][tab][tab] | [tab][tab] [any layout]

Moving the mouse from screen B to the "firefox" tab section on the left would often get focus stuck on the "other" tab, and I was often closing the wrong window when intending to close a browser window. I did generally have my 9 workspaces filled with windows and updating layouts takes a noticeable amount of time. If I understand it right, the not-visible windows still slow down the recomputations.

...

...

Probably the right thing to do would be to clear the pending crossing events in 'windows' whenever the layout manipulation moves/resizes/remaps windows, and not clear it if only the focus has changed. What would be a clean way to make that distinction?

Probably a better option would be to not clear crossing events at all, but instead when we handle crossing events to first check if the mouse is still in the said window. If it's not, then we should ignore the event.

That sounds reasonable - how about a something like the following (not a real patch - I'm still away from my computer including next week and can't test this):

XMonad/Main.hs: handle e@(CrossingEvent {ev_window = w, ev_event_type = t}) | t == enterNotify && ev_mode e == notifyNormal && ev_detail e /= notifyInferior - = whenX (asks $ focusFollowsMouse . config) (focus w) + = whenX (asks $ focusFollowsMouse . config) $ do + dpy <- asks display + root <- asks theRoot + (_,_,w',_,_,_,_,_) <- io $ queryPointer dpy root + when (w == w') (focus w)

XMonad/Operations.hs: mapM_ (flip setWMState withdrawnState) (W.allWindows old \\ W.allWindows ws) - - unless isMouseFocused $ clearEvents enterWindowMask

The change should be noticeable if you put a large number of windows on the screen and quickly move the mouse across them. The three behaviors to look for would be:

- focus stays in one of the first windows entered (before the mouseFocused patch)

- each window crossed briefly gets focused (the border flashes), with focus staying in the final one (with mouseFocused patch)

- the final window gets focused, and maybe one or two of the ones crossed along the way, but most skip getting focused along the way (hopefully the new behavior with the above change)

-Klaus _______________________________________________ xmonad mailing list xmonad@haskell.org http://www.haskell.org/mailman/listinfo/xmonad

On 2008 May 17, at 14:38, Spencer Janssen wrote:

On Sat, May 17, 2008 at 05:09:41AM -0700, David Roundy wrote:

...

Probably a better option would be to not clear crossing events at all, but instead when we handle crossing events to first check if the mouse is still in the said window. If it's not, then we should ignore the event.

This misses the entire reason why we clear window entry events in the first place. When the current layout changes (or when switching workspaces) a window entry event will be generated for the window under the cursor even if the mouse is entirely stationary. We don't want to perform focus changes based on the position of the mouse if the user isn't even moving it.

Maybe I'm confused, but I think you just said the same thing he did. -- brandon s. allbery [solaris,freebsd,perl,pugs,haskell] allbery@kf8nh.com system administrator [openafs,heimdal,too many hats] allbery@ece.cmu.edu electrical and computer engineering, carnegie mellon university KF8NH

On Sat, May 17, 2008 at 12:13:00PM -0700, Klaus Weidner wrote:

On Sat, May 17, 2008 at 01:38:48PM -0500, Spencer Janssen wrote:

...

On Sat, May 17, 2008 at 05:09:41AM -0700, David Roundy wrote:

...

On Sat, May 17, 2008 at 03:12:49AM -0700, Klaus Weidner wrote:

...

Probably the right thing to do would be to clear the pending crossing events in 'windows' whenever the layout manipulation moves/resizes/remaps windows, and not clear it if only the focus has changed. What would be a clean way to make that distinction?

Probably a better option would be to not clear crossing events at all, but instead when we handle crossing events to first check if the mouse is still in the said window. If it's not, then we should ignore the event.

This misses the entire reason why we clear window entry events in the first place. When the current layout changes (or when switching workspaces) a window entry event will be generated for the window under the cursor even if the mouse is entirely stationary. We don't want to perform focus changes based on the position of the mouse if the user isn't even moving it.

So would the right thing be to do a combination of both? Keep the current mouseFocused logic in Operations.hs to discard crossing events if the user wasn't moving the mouse, and add the check in Main.hs to skip the focusing on Enter events if the pointer isn't in the window being entered anymore?

-Klaus

Maybe this will work. Though I worry about race conditions -- remember that X11 is an asynchronous protocol.