GNOME Bugzilla – Bug 753256
Don't handle key event when nothing happens
Last modified: 2015-08-05 15:17:40 UTC
Created attachment 308740 [details] [review] Add/remove scroll binding when needed It can happen that when a key event which controls a scrollbar is triggered nothing actually happens because the scrollbar is not shown. Unfortunately, GtkScrolledWindow still proceeds this event and "gtk_window_propagate_key_event()" returns that the event was handled. This causes a problem in evince when rotating document using Ctrl+{Left, Right} sometimes because it also moves horizontal scrollbar in GtkScrolledWindow. Evince uses a custom handling of key press event (see https://git.gnome.org/browse/evince/tree/shell/ev-window.c?id=3.17.4#n5614) which thinks that the event was handled so it doesn't send it further but it actually was not processed. Attached patch adds/removes the scroll bindings as needed.
This would lead to rotate left/right keybindings which only work sometimes. Is that really desired ?
It was my goal to have the rotation working at least in the situation when the scrollbar is not shown. It does nothing now so it seems that the event was not handled (when the scrollbar is not visible). I can ask at evince mailing list if you wish to have an opinion of evince's developers. But if you think that the patch is not appropriate then close it as notabug or wontfix.
I think it makes sense to not handle the events if the scrollbar is turned off altogether, using policy never. Not sure if I would do the same if the content just 'happens to fit'. In any case, I would probably keep the bindings installed, and just do the filtering in the scroll_child function.
Created attachment 308768 [details] [review] Don't handle key event when can't scroll (In reply to Matthias Clasen from comment #3) > I think it makes sense to not handle the events if the scrollbar is turned > off altogether, using policy never. Not sure if I would do the same if the > content just 'happens to fit'. In any case, I would probably keep the > bindings installed, and just do the filtering in the scroll_child function. Here is a modified patch which does the filtering in the scroll_child function.
Review of attachment 308768 [details] [review]: looks good to me now
Comment on attachment 308768 [details] [review] Don't handle key event when can't scroll Thank you for the review, I've pushed the patch to master.