jD91mZM2 / xidlehook

GitLab: https://gitlab.com/jD91mZM2/xidlehook
MIT License
387 stars 33 forks source link

--not-when-fullscreen... again #23

Closed jD91mZM2 closed 4 years ago

jD91mZM2 commented 5 years ago

I'm having a similar issue, I think. If Firefox is fullscreen it will still trigger the screenlock. If audio is running it will still trigger screen dimming but not lock.

#! /bin/bash

# Run xidlehook
xidlehook \
  `# but not when fullscreen` \
  --not-when-fullscreen \
  `# and not when audio playing` \
  --not-when-audio \
  `# dim after 60 seconds inactivity, undim if activity` \
  --timer normal 60 \
    'xrandr --output HDMI2 --brightness .4 --output VGA1 --brightness .4' \
    'xrandr --output HDMI2 --brightness 1 --output VGA1 --brightness 1' \
  `# after ten seconds undim and run gllock` \
  --timer primary 15 \
    'xrandr --output HDMI2 --brightness 1 --output VGA1 --brightness 1; gllock' \
    '' \
  `#if no activity in another 30 seconds suspend `\
    --timer normal 60 \
      'systemctl suspend'\
      ''

Originally posted by @stevensonmt in https://github.com/jD91mZM2/xidlehook/issues/5#issuecomment-504781615

jD91mZM2 commented 5 years ago

What DE/WM are you using? This has occured to me in xmonad due of a lack of EWMH hints.

To debug this, please run the following command and click a fullscreen window.

xprop | grep _NET_WM_STATE

EDIT: Oh, and xidlehook version would be great!

stevensonmt commented 5 years ago

WM is leftwm. Will update with other info when I get home again, but I did check that xprop showed the _NET_WM_STATE_FULLSCREEN property before adding my comment.

stevensonmt commented 5 years ago
~/ xprop | grep _NET_WM_STATE
_NET_WM_STATE(ATOM) = _NET_WM_STATE_FULLSCREEN, _NET_WM_STATE_MAXIMIZED_HORZ, WM_COLORMAP_WINDOWS
~/ xidlehook --version
xidlehook 0.7.1
jD91mZM2 commented 5 years ago

I assume the window is the selected one? See #19

stevensonmt commented 5 years ago

yes, it is the focused window.

jD91mZM2 commented 5 years ago

I'm afraid I don't have much time to debug this at the moment, especially since I can't reproduce it :(

If you'd like, you can spray some dbg!()s everywhere in https://github.com/jD91mZM2/xidlehook/blob/152d057a89a1be9b1fabf38e632efd779a725183/src/main.rs#L416. Once you reach a conclusion to what the issue is, I'll be happy to reproduce the bug for myself and fix it.

stevensonmt commented 5 years ago

I'm going to be out of touch for the next week or so but when I get back I'll give it a look and see if I can figure it out. Thanks.

stevensonmt commented 5 years ago

Just wondering if you could help me understand this bit of code from x11api.rs:

    // Get the window properties of said window
    let mut data: *mut u8 = ptr::null_mut();
    let mut nitems = 0;
    XGetWindowProperty(
        display,
        focus,
        XInternAtom(display, NET_WM_STATE.as_ptr() as *const c_char, 0),
        0,
        c_long::max_value(),
        0,
        XA_ATOM,
        ignored_ulong,
        ignored_int,
        &mut nitems,
        ignored_ulong,
        &mut data
    );

    let data = XIntPtr(data as *mut _);
    let atom = XInternAtom(
        display,
        NET_WM_STATE_FULLSCREEN.as_ptr() as *const c_char,
        0
    );

    // Check the list of returned items for _NET_WM_STATE_FULLSCREEN
    for i in 0..nitems as isize {
        if *data.0.offset(i) == atom {
            return Ok(true);
        }
    }

I don't see where nitems is incremented. If it is not incremented does the check never happen?

jD91mZM2 commented 5 years ago

Of course!

Both nitems and data are set by XGetWindowProperty, assuming the call succeeds. I should actually have some error handling there, I guess.

A good sanity check would be to add dbg!(nitems) somewhere after that call to make sure it prints out 3 for a fullscreen window. Then print out the element values and the value of atom which is what it's searching for.

stevensonmt commented 5 years ago

So I've tried with chromium and firefox running fullscreen. With chromium fullscreen dbg!(nitems) returns nitems = 2 (not at fullscreen it returns nitems = 1), but with firefox in either fullscreen or not I get nitems = 0. In all cases atom = 349.

The above is with leftwm. With XFCE firefox at fullscreen also returns nitems = 2.

I only get nitems = 3 if the terminal window running xidlehook is at fullscreen or even if it is just maximized. Otherwise results are very inconsistent. In this screenshot firefox is fullscreen the entire time and nitems goes from 2 to 0 for some reason. screenshot It later went back to 2 and then back to 0 depending on which window had focus (terminal focus => 2, FF focus => 0) regardless of the FF fullscreen status.

With chromium in XFCE I get nitems of 3 for maximized or 4 for fullscreen after maximized. I get 2 if not maximized or fullscreen but 3 if fullscreen from a non-maximized state.

jD91mZM2 commented 5 years ago

Amazingly debugged :)! When you ran xprop | grep _NET_WM_STATE, did you make sure Firefox was fullscreen and focused?

stevensonmt commented 5 years ago

Not sure, but just did it again and got

>> sleep 3 && xprop | grep _NET_WM_STATE
_NET_WM_STATE(ATOM) = _NET_WM_STATE_MAXIMIZED_VERT, _NET_WM_STATE_FULLSCREEN, _NET_WM_STATE_FOCUSED

The sleep was so I could use the mouse to focuse FF before xprop ran. The output was the same when chromium is fullscreen and focused.

That was with XFCE. With leftwm the output is less verbose (but again identical for FF and chromium):

>>sleep 3 && xprop | grep _NET_WM_STATE
_NET_WM_STATE(ATOM) = _NET_WM_STATE_FULLSCREEN
jD91mZM2 commented 5 years ago

What's the output if you replace

if *data.0.offset(i) == atom {

with

if dbg!(*data.0.offset(i)) == atom {

?

stevensonmt commented 5 years ago

Zeroes with firefox fullscreen or not, focused or not. 1 with chromium fullscreen, focused or not. screenshot

jD91mZM2 commented 5 years ago

Only thing I can think of is wrapping those XGetInputFocus(...) and XGetWindowProperty(...) calls in dbg!, in case one of them is returning an error. Otherwise, none of this output makes sense to me :(

stevensonmt commented 5 years ago

What output you would expect?

jD91mZM2 commented 5 years ago

So xprop admits that yes, there are a bunch of values for the property _NET_WM_STATE, and one of these values are the atom _NET_WM_STATE_FULLSCREEN. But when xidlehook lists the same property, somehow it claims to have zero length. So the only thing I can think of is if the API invocation is failing and it uses zero as the default value.

In case I misunderstood, and you're asking what return value I expect from the two functions I asked you to wrap, 0 is the standard in C to specify success. Non-zero usually means failure, and a negative value almost certainly means failure. But what type of failure is interesting!

stevensonmt commented 5 years ago

In leftwm, xprop for non-fullscreen window is empty for that property:

xprop | grep _NET_WM_STATE
_NET_WM_STATE(ATOM) =

Also I may have inadvertently introduced some confusion. I was trying to figure out what the XGetWindowProperty call was doing and had it print the return of that which is where that extra zero in the screenshots comes from.

Wrapping XGetWindowProperty in dbg returns 0, but XGetInputFocus returns 1 no matter what window is focused. This is true for both leftwm and XFCE.

jD91mZM2 commented 5 years ago

I was trying to figure out what the XGetWindowProperty call was doing and had it print the return of that which is where that extra zero in the screenshots comes from.

I ignored that actually, but that's useful. Seems like XGetWindowProperty isn't failing at least... What's the return value of XGetInputFocus?

stevensonmt commented 5 years ago

XGetInputFocus returns 1 no matter which window is focused in both leftwm and XFCE. I must have edited my earlier comment while you were replying, sorry.

jD91mZM2 commented 5 years ago

Interesting, and highly useful. Unless I'm mistaken, that 1 means BadRequest. I'll take a look at what the values are on my machine. Sorry for the late reply

jD91mZM2 commented 5 years ago

Nope, same on my system. When googling for how to handle errors I stumbled across a rant about Xlib, and rust-xcb which is a safe API. Guess that module is due for a rewrite... Be right back!!

jD91mZM2 commented 5 years ago

Update: The xcb change is out, available at 7faf3e0d1112059d8b34ffe812e257062b778f3e. Xcb is amazing and rust-xcb is even more so.

I'm hopeful that, if anything, this will work for you. But no promises. At least we might get better error handling by default, which could lead us a step on debugging. So please do try this commit :D

stevensonmt commented 5 years ago

Will have to spend some more time looking into it, but for now it does not seem to recognize a fullscreen window whether it is the focused window or not.

jD91mZM2 commented 5 years ago

Not even for things that previously worked, like chromium (I think you mentioned?)

vevais commented 5 years ago

Hello, I am having the same problem using awesome v4.3 with xidlehook 0.7.0 and trying to watch movies in fullscreen in either YouTube or Netflix. This is my script:

#! /bin/bash
xidlehook \
  --not-when-fullscreen \
  --timer normal 90 \
    'xbacklight -set 0' \
    'xbacklight -set 100' \
  --timer normal 3600 \
    'xbacklight -set 100 && systemctl suspend' \
    ''

That's the output of xprop | grep _NET_WM_STATE for both YouTube and Netflix fullscreen windows while active: _NET_WM_STATE(ATOM) = _NET_WM_STATE_FULLSCREEN, _NET_WM_STATE_MAXIMIZED_VERT, _NET_WM_STATE_MAXIMIZED_HORZ

Tell me if you need more information.

jD91mZM2 commented 5 years ago

Thanks! Would you be so kind as to send me the output of xidlehook? Turns out I left a dbg! there and I suppose that's for the best.

What happens if you get the latest git version and run

cargo run --bin xidlehook -- --timer normal 5 "echo hi" "" --not-when-fullscreen

and then try experimenting with and without fullscreen?

vevais commented 5 years ago

Hmm, really weird. It gives me [src/x11api.rs:54] prop.value() = [] no matter which window is focused, fullscreen or not. The only real output is given when the window I run the program from is focused:

[src/x11api.rs:54] prop.value() = [
    383,
    387,
]
jD91mZM2 commented 5 years ago

Oh. It only checks for fullscreen status on active windows - that's a feature... or lack thereof, I guess. Feel free to drop a comment on #19 if you want that expanded.

vevais commented 5 years ago

Sorry, but I'm at a loss here. Maybe I misunderstood something? I would be happy if it (correctly) checked for fullscreen status on active windows such as Netflix or YouTube, which is not the case.

jD91mZM2 commented 5 years ago

My bad, it is indeed supposed to check fullscreen status of other active windows. I must have misread the following:

The only real output is given when the window I run the program from is focused

I thought you meant when the window that's fullscreen is focused. Did you mean the only output that occurs is when you run xidlehook in a terminal and that terminal is fullscreen?

EDIT: Your two values for _NET_WM_STATE were _GTK_EDGE_CONSTRAINTS and EDID. You can look this up using https://gist.github.com/jD91mZM2/3d26f52a774d5a3ed5f0c94f2c03d562. Can you verify for me that under the same conditions, xprop | grep "^_NET_WM_STATE" return those values?

karlb commented 4 years ago

I also get unwanted screen locks when using FF to watch netflix with xidlehook 0.7.1 (using --not-when-fullscreen, of course). Window manager is dwm.

xprop says _NET_WM_STATE(ATOM) = _NET_WM_STATE_FULLSCREEN.

jD91mZM2 commented 4 years ago

What do the debug logs say? Run the latest xidlehook with export RUST_LOG=trace and paste the output in gist or something.

karlb commented 4 years ago

What do the debug logs say? Run the latest xidlehook with export RUST_LOG=trace and paste the output in gist or something.

For this test I started xidlehook 0.8.1, switched to a workspace with a fullscreen firefox and waited for the timer to trigger.

karl@t480k:~$ xidlehook --not-when-fullscreen --timer 5 'xset dpms force off' ''
[2020-01-05T15:38:06Z INFO  async_std::task::block_on] block_on
[2020-01-05T15:38:06Z TRACE xidlehook_core] Taking the first timer into account. Remaining: 5s
[2020-01-05T15:38:06Z TRACE xidlehook_core] Relative time: 8ms
[2020-01-05T15:38:06Z TRACE xidlehook_core] Taking next timer into account. Remaining: 4.992s
[2020-01-05T15:38:06Z TRACE xidlehook_core] Sleeping for 4.992s
[2020-01-05T15:38:11Z TRACE xidlehook_core] Taking the first timer into account. Remaining: 5s
[2020-01-05T15:38:11Z TRACE xidlehook_core] Relative time: 2.659s
[2020-01-05T15:38:11Z TRACE xidlehook_core] Taking next timer into account. Remaining: 2.341s
[2020-01-05T15:38:11Z TRACE xidlehook_core] Sleeping for 2.341s
[2020-01-05T15:38:14Z TRACE xidlehook_core] Taking the first timer into account. Remaining: 5s
[2020-01-05T15:38:14Z TRACE xidlehook_core] Relative time: 5.001s
[2020-01-05T15:38:14Z TRACE xidlehook_core] Activating timer 0
[2020-01-05T15:38:14Z DEBUG xidlehook_core::modules::xcb] xcb::xproto::get_property(...) = []
[2020-01-05T15:38:14Z DEBUG xidlehook_core::modules::xcb] NET_WM_STATE_FULLSCREEN = 331
[2020-01-05T15:38:14Z TRACE xidlehook_core] Sleeping for 5s
[2020-01-05T15:38:19Z TRACE xidlehook_core] Taking the first timer into account. Remaining: 5s
[2020-01-05T15:38:19Z TRACE xidlehook_core] Relative time: 111ms
[2020-01-05T15:38:19Z TRACE xidlehook_core] Taking next timer into account. Remaining: 4.889s
[2020-01-05T15:38:19Z TRACE xidlehook_core] Sleeping for 4.889s
^C[2020-01-05T15:38:23Z INFO  async_std::task::block_on] block_on
[2020-01-05T15:38:23Z INFO  async_std::task::block_on] completed
[2020-01-05T15:38:23Z INFO  async_std::task::block_on] completed
[2020-01-05T15:38:23Z TRACE xidlehook] Signal received: SIGINT
jD91mZM2 commented 4 years ago

Damn interesting, so I guess the question is reduced to: why would xcb::xproto::get_property ever be [] if xprop can still see the property?

jD91mZM2 commented 4 years ago

Of course, xprop uses the exact same call I've used previously...

Idk, have you tried if version 0.7.1 works? It's using the X call directly, no XCB middleman. I mean it's unlikely, but I don't know how to proceed here, the docs on that function are pretty bad.

karlb commented 4 years ago

Idk, have you tried if version 0.7.1 works?

Yes, it has the same problem. I originally had the problem with 0.7.1 and only updated to provide up to date debugging info for this ticket.

jD91mZM2 commented 4 years ago

Update: I experience this after switching to AwesomeWM! I can finally reproduce & attempt to fix it

jD91mZM2 commented 4 years ago

From my side, the error happens because the active window is actually different from the fullscreen window. To fix this, xidlehook would need to check all windows.

jD91mZM2 commented 4 years ago

Alright, does the latest master work as you'd expect? I made it check all windows instead