-
Notifications
You must be signed in to change notification settings - Fork 192
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
assert_and_click: handle undefined previous mouse position #559
Conversation
assert_and_click tries to record the previous mouse cursor position and return the cursor there after clicking. However if the mouse has never been explicitly positioned anywhere before, `backend_get_last_mouse_set` returns undefined for each co-ordinate, which results in a couple of 'use of uninitialized value in int' warnings and the cursor being set to 0,0, which may trigger the GNOME overview under Wayland. So catch when the values are undefined and in this case, call mouse_hide() - which puts the cursor at bottom-right - instead. This avoids both the warnings and the overview trigger. Also fix another uninitialized value warning in `mouse_hide` while we're at it - check if border_offset is defined before using it.
788d78f
to
400a02e
Compare
@@ -188,9 +188,11 @@ sub mouse_hide { | |||
$self->{mouse}->{x} = $self->{vnc}->width - 1; | |||
$self->{mouse}->{y} = $self->{vnc}->height - 1; | |||
|
|||
my $border_offset = int($args->{border_offset}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could have fixed this by just passing a || 0 within the int cast - the -= 0 wouldn't have hurt.
But of course your version is spelling it out with more whitespace :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I dunno, it just feels better to me this way. either works.
I suppose there's one possible risk here which is that moving the mouse to bottom-right may trigger a tooltip on some desktops with a traditional bottom panel, which may interfere with some needle match...I'll see if that happens on any Fedora tests, but you (SUSE) probably have a lot more than us which may be affected. |
I think this is a 'safe' change (or to put it another way, I think @AdamWill is worrying too much ;)) we use mouse_hide all the time and I can never think of a time it causes a tooltip or hotcorner activation, makes sense to me to use it if assert_and_click has nowhere else to go. |
just for amusement's sake - I just realized this exact patch caused a problem I've been banging my head against for weeks: https://openqa.fedoraproject.org/tests/32777 In that test (and a few others) we run Firefox in a bare X session (no window manager at all, just Now I know what the bug is I can work around it easily, of course. Just thought it was funny that it drove me nuts for weeks and I spent all last night fiddling around to try and figure it out and it turned out to be my own damn fault all along... |
glad you like your own changes ;-) |
assert_and_click tries to record the previous mouse cursor
position and return the cursor there after clicking. However if
the mouse has never been explicitly positioned anywhere before,
backend_get_last_mouse_set
returns undefined for eachco-ordinate, which results in a couple of 'use of uninitialized
value in int' warnings and the cursor being set to 0,0, which
may trigger the GNOME overview under Wayland. So catch when the
values are undefined and in this case, call mouse_hide() - which
puts the cursor at bottom-right - instead. This avoids both the
warnings and the overview trigger.