Resubmit #128 (X.H.Focus)

[sgf-dma]

Resubmit #128 with changes requested in #162 .

@pjones , @geekosaur Please, test and review.

@SirBoonami , @unclechu Please, try updated version. Now logHook (instead of manageHook) runs for activated window. Thus, if you use this module for managing focus only for activated windows, you need to add FocusHook to your logHook. E.g.

logHook = activateLogHook activateSwitchWs

If you use FocusHook for managing focus for both new and activated windows, you need to add it to both. Either create two different FocusHook-s, e.g.:

let newFh :: ManageHook
    newFh = manageFocus newFocusHook
    acFh :: X ()
    acFh = activateLogHook (manageFocus activateFocusHook)
    xcf = ewmh $ def
                 { manageHook   = newFh <+> manageHook def
                 , logHook      = acFh  <+> logHook def
                 }

or (as was before) use the single FocusHook with activated predicate, e.g.:

let fh :: ManageHook
    fh = manageFocus $ (composeOne
            [ liftQuery activated -?> activateFocusHook
            , Just <$> newFocusHook
            ])
    xcf = ewmh $ def
                 { manageHook   = fh <+> manageHook def
                 , logHook      = activateLogHook fh <+> logHook def
                 }

See description for more examples and details.

Checklist

• I've read CONTRIBUTING.md

• I tested my changes with xmonad-testing

• I updated the CHANGES.md file

[unclechu]

@sgf-dma works as expected, I just updated to last commit without any changes in my config code:
https://github.com/unclechu/xmonadrc/tree/6928ecf74e0221c4cd855cf1f7ecacd7beed9fd2/xmonad

[sgf-dma]

@unclechu Thanks. But after looking into your config i can't find where you're enabling window activation? Does window activation works at all in your config?

If you use FocusHook only for managing focus of new windows, that explains why no changes were necessary (but then your activateFocusHook in https://github.com/unclechu/xmonadrc/blob/master/xmonad/src/FocusHook.hs never runs), but if not - that's strange.

Starting from commit 8278cf6 (which was in previous version too; the commit, where handleFocusQuery function was removed) you need to use ewmh from X.H.EwmhDesktops to enable window activation.

[unclechu]

@sgf-dma I have everything about focus-hook here.

only for managing focus of new windows

Maybe I misunderstand something, what do you mean by "managing focus of NOT new windows"? What I actually need (and already got) is:

1. Prevent losing focus from windows of specific applications;
2. Some applications should have hard priority for grabbing focus and should be able steal the focus notwithstanding item 1;
3. Some applications creates child dialog windows, focus should move to dialog window if it belongs to same application;
4. Hardcore mode for grabbing focus for current focused window.

[sgf-dma]

I mean, that window may be either new or activated using _NET_ACTIVE_WINDOW message. For using FocusHook for new windows, you need to add it to manageHook as you did. But for using FocusHook for activated windows, you first need to enable window activation using ewmh function and then add activateLogHook .. to your logHook, because ewmh will call logHook for handling focus upon window activation.

ewmh default behavior in all previous xmonad version was to switch workspace and focus to activated window. Now it does nothing. You need to explicitly write what you want to happen on window activation using activateLogHook (as described in comments in X.H.EwmhDesktops).

I test window activation with running firefox google.ru in console. According to your activateFocusHook it should switch to workspace with firefox in any case and focus firefox window, unless one of conditions from a list passed to keepFocusFor succeeds. If that doesn't happen, you probably does not have enabled window activation support. Or you may try running

xprop -root _NET_SUPPORTED

ewmh should add _NET_ACTIVE_WINDOW there.

Also, take note, that:

• focused and focusedCur predicates are not the same. As described in comments in X.H.Focus:

  activate all windows, unless gnome-terminal is focused on current workspace:

           let mh :: ManageHook
               mh  = manageFocus (not <$> focusedCur (className =? "Gnome-terminal")
                       --> liftQuery activateSwitchWs)
  

  or activate all windows, unless focused window on the workspace, where activated window is, is not a gnome-terminal:

  >         let mh :: ManageHook
  >             mh  = manageFocus (not <$> focused (className =? "Gnome-terminal")
  >                     --> liftQuery activateSwitchWs)
  

  Because you've used focused, you'll go the second way.

• Your keepFocusFor function will keep focus on a windows in a list, but will not prevent workspace switch (requested by default action at the end), because keepFocus <+> switchWorkspace - will switch workspace and keep focus on a workspace, you've switched to.

[unclechu]

@sgf-dma Sometimes I'm having a bug with this hook, when I have started some application and opened gmrun and later window of previously started application appeared, - focus stays on gmruns window but input text field inside this gmruns window loses its focus. I can heal it any time by forcing refresh (from XMonad.Operations) by pressing mod+r for example. Here. Also my own tool place-cursor-at in similar case stops handling pressed keys unit I press mod+r or switch focus to another window.

This bug triggered not by any application. For example by any electron-based application or by starting chromium-browser.

[sgf-dma]

Yes, there is such bug. But, if we're talking about the same thing, this is a
bug not in my module, but in xmonad. It's reproducible for me with def
config, when firefox window is activated. I.e. with following config

import XMonad
import XMonad.Util.EZConfig

-- | Broken.
main = do
    xmonad $ def {modMask = mod4Mask}
      `additionalKeys` [((mod4Mask, xK_r), refresh)]

open a terminal (i use xterm) and firefox window at the same workspace, and
then run firefox google.ru in the terminal. This triggers activation of
firefox window and input focus will switch to firefox, but window focus (red
border) will remain on terminal. Running refresh will fix input focus
(return back to terminal).

And the fix for this is to enable window activation, i.e. to call ewmh
function:

import XMonad
import XMonad.Util.EZConfig
import XMonad.Hooks.EwmhDesktops

-- | Working.
main = do
    xmonad . ewmh $ def {modMask = mod4Mask}
      `additionalKeys` [((mod4Mask, xK_r), refresh)]

with this config input focus will not be grabbed by activated window.

Note, that after my PR window activation by default will do nothing, so you may safely
enable it (well, if you're not using desktopConfig, where i've preserved old
behavior - switch to workspace and switch focus to activated window).

I can't run your config without some removals (i've installed only xmonad
binary), but with following diff activated firefox window does not grab input
focus with your config:

diff --git a/xmonad/src/Main.hs b/xmonad/src/Main.hs
index dfc6192..fff894d 100644
--- a/xmonad/src/Main.hs
+++ b/xmonad/src/Main.hs
@@ -26,6 +26,8 @@ import Config (myConfig)
 import Keys (myKeys)
 import Utils (xmobarEscape)
 import Utils.IPC (initIPC, deinitIPC)
+import XMonad hiding (keys)
+import XMonad.Hooks.EwmhDesktops
 import Utils.CustomConfig
   ( getCustomConfig
   , Config ( cfgInactiveWindowOpacity
@@ -43,26 +45,10 @@ main = do
   let conf = myConfig customConfig
       keys = myKeys ipc myWorkspaces customConfig

-  xmproc <- spawnPipe "xmobar ~/.xmonad/xmobar.generated.hs"
-
-  xmonad $ conf
-    { logHook = do
-
-        DL.dynamicLogWithPP $ def
-          { DL.ppOutput  = hPutStrLn xmproc
-          , DL.ppTitle   = DL.xmobarColor "gray" "#444" . DL.wrap " " " "
-          , DL.ppCurrent = DL.xmobarColor "green" ""    . DL.wrap "[" "]"
-          , DL.ppSep     = "  "
-          , DL.ppWsSep   = " "
-          , DL.ppLayout  = DL.xmobarColor "yellow" "" . layoutNameHandler
-          , DL.ppHiddenNoWindows = id
-          }
-
-        let inactiveOpacity = cfgInactiveWindowOpacity customConfig
-         in if cfgInactiveWindowOpacityOnlyForCurrentWs customConfig
-               then fadeInactiveCurrentWSLogHook inactiveOpacity
-               else fadeInactiveLogHook inactiveOpacity
+  --xmproc <- spawnPipe "xmobar ~/.xmobarrc"

+  xmonad . ewmh $ conf
+    {  modMask = mod4Mask
     } `additionalKeys` keys

   deinitIPC ipc

Please, try this and if this fixes your config, i'll submit a separate bug
report about this behavior to xmonad.

[geekosaur]

On Sun, Jun 25, 2017 at 11:45 AM, sgf-dma ***@***.***> wrote: open a terminal (i use xterm) and firefox window at the same workspace, and then run firefox google.ru in the terminal. This triggers activation of firefox window and input focus will switch to firefox, but window focus (red border) will remain on terminal. Running refresh will fix input focus (return back to terminal). And the fix for this is to enable window activation, i.e. to call ewmh function:

Firefox back to violating ICCCM again? Figures. We're back to needing a way to lie about EWMH support again, then, to stop this crap. Briefly: firefox is saying "oh, the WM isn't advertising EWMH, I'll just forcibly steal the focus for myself because I feel like it". We don't handle that well because programs are not supposed to do it, and justifiably the correct response is not to update the border but to switch the focus back to where the user had it in the first place.

…

-- brandon s allbery kf8nh sine nomine associates allbery.b@gmail.com ballbery@sinenomine.net unix, openafs, kerberos, infrastructure, xmonad http://sinenomine.net

[unclechu]

@sgf-dma by adding ewmh loosing focus issue have been fixed, but item 2 of my wish-list (that I pasted above) was broken by that (gmrun couldn't grab focus from windows that keeps their focus). As you mentioned earlier I should have add log hook (activateLogHook focusManageHook), so I did it and it fixed the item 2. Now it's completely solved! Thanks a lot for your help and contribution!

[unclechu]

@sgf-dma oh, I just realized that now when I open some link for example by running command xdg-open https://duckduckgo.com it switches focus to Firefox window and switches workspace where it is if it's not on any of screens already. Could I disable this behavior?

[sgf-dma]

I've glanced over changes you've made in your config.. well, removing
logHook (as in diff in my comment) is not necessary at all. I did this just
because it was the simpler way (for me) to run your config. The real fix is to
call ewmh only.

when I open some link for example by running command xdg-open
https://duckduckgo.com it switches focus to Firefox window and switches
workspace where it is if it's not on any of screens already. Could I
disable this behavior?

Of course, you can. It's doing exactly what you've asked it to:

activateFocusHook :: FocusHook
activateFocusHook = composeAll $
  keepFocusFor [ className =? "Gmrun"

               , className =? "Firefox"
               , className =? "Tor Browser"

               -- Prevent lost focus for all messangers
               , className =? "Gajim"
               , className =? "Hexchat"
               , className =? "utox"
               , className =? "qTox"
               , className =? "Gnome-ring"
               , className =? "Riot"
               , className =? "Rambox"

               , className =? "Keepassx"
               , title     =? "gpaste-zenity"
               ]
  ++
  [ return True --> switchWorkspace <+> switchFocus ]
  where keepFocusFor = foldr ((:) . f) []
        f cond = focused cond --> keepFocus

Essentially, if a window in the list is focused on workspace, where activated
window is (may not be current workspace), you have here

keepFocus <+> switchWorkspace <+> switchFocus

which is equal to just switchWorkspace. If some other window is focused, on
workspace, where activated window is, you have

switchWorkspace <+> switchFocus

Before enabling ewmh this FocusHook just never runs. So, the simplest fix
is to just do nothing, when window is activated. Either not add your
FocusHook to logHook (remember, to fix bug with input focus you need only
ewmh), or run idHook for activated window:

focusManageHook :: ManageHook
focusManageHook = manageFocus
                $ composeOne [ liftQuery activated -?> mempty
                                , Just <$> mempty ]

or make something different with activated window, e.g. move it to current
workspace (see activateOnCurrentWs and activateOnCurrentKeepFocus). But
note, that to see activated window on current workspace in FocusHook, you need
to move it in a separate FocusHook, like

activateLogHook (focusManageHook <+> activateOnCurrentWs)

See module description for more details and full example.

[unclechu]

@sgf-dma

I've glanced over changes you've made in your config.. well, removing
logHook (as in diff in my comment) is not necessary at all. I did this just
because it was the simpler way (for me) to run your config. The real fix is to
call ewmh only.

I didn't remove it, I just moved it to separate module.

[unclechu]

@sgf-dma sorry, I just totally forgot about I have this exactly feature described in my config, just because it haven't work before I add ewmh. Thanks again. Fixed by unclechu/xmonadrc@4ba4020

[SOwOphie]

Sorry for being utterly late to the party. Your module seems reasonable, the documentation is thorough. I didn't get to test everything, but my test cases

• switchWorkspace for everybody
• ignore all
• ignore blacklisted, switchWorkspace for the rest

were easy to implement and worked like a charm.

[sgf-dma]

@byorgey @ivanbrennan @geekosaur Can we finally merge this? Or someone still has questions about this module behavior?

[byorgey]

@sgf-dma I don't see why not. Would you mind taking a look at the merge conflicts?

[sgf-dma]

Rebased. Please, review.

[byorgey]

Thanks!

[liskin]

I guess I'm later to the party than everyone else, and I'm sorry (my excuse being that I had a working version of this in #110 so I didn't have the motivation to fix what already worked, but now that it's merged I need to adapt my config so I'm forced to look at this).

Why does this invoke the global logHook (or the global manageHook in the previous version of the code) instead of ewmh taking the focusHook as a parameter and using that in its ewmhDesktopsEventHook? This results in the logHook being invoked twice when window activation is requested, which is ugly (but yeah, objectively not that big a deal). Alternatively, ewmh could still take no parameters and invoke a new ewmhFocus function with a default focusHook that retains the original behaviour. This would be nice as there won't be a breaking change that way. We can even add some sort of EwmhConfig record type with focusHook and handleFullscreen to make EWMH behaviour easier to configure. I'd prefer this over putting this stuff in a logHook, where it doesn't belong.

Furthemore, there's one little bit missing in this that's solved in #110: differentiating between windows asking to be activated and pagers (alttab, rofi, …) requesting that a window is activated. See https://github.com/xmonad/xmonad-contrib/pull/110/files#diff-cf9308345d1d5de4c1e403c5874d91d723ffc5d4ff79b21c4dbfe6390a592598R236. This bit will need to be fixed before I can switch over to this.

Let me know what you folks think. I can implement any or all of this, but we should first agree if these changes are desired or whether you're all happy with the current design. I'm Liskni_si in #xmonad@freenode if you wish to discuss this more interactively.