GNOME Bugzilla – Bug 78669
with hidden cursor, keynav shouldn't be cursor-based
Last modified: 2011-02-04 16:12:26 UTC
If the cursor is hidden in a GtkTextView and you try to scroll up or down, you first have to move the _invisible_ cursor to the top or bottom line before the text actually moves. Most users (at least me) might not understand the importance of the concept of an invisible cursor. Is this known or already fixed?
It is known, but I'm not sure we had an open bug, and I don't think it's fixed yet.
Found this one sitting on a past milestone.
Move GtkTextView 2.0.4 bugs to 2.0.5
Moving bugs from older 2.0.x milestones to 2.0.10.
Here is an attempt to implement this. Maybe it would be cleaner to have two separate sets of bindings, one for cursor-based navigation and one for viewport-based navigation ...
Created attachment 13788 [details] [review] patch
GTK_MOVEMENT_PAGES is already viewport-based, right? though I suppose that's a bit strange since the signal is called move_cursor. This patch seems good for 2.2, but for 2.4 maybe a slightly different patch that mops up PAGE_HORIZONTALLY_HACK_VALUE, and adds GTK_MOVEMENT_SCROLL_STEP (or something - need better name), so you can bind it even for editable widgets. Then at the top of move_cursor_internal could just do "if (!cursor_visible) { step = GTK_MOVEMENT_SCROLL_STEP }" to replace the cursor-based step with the viewport one. Two binding sets does seem like overkill to me.
Yes, GTK_MOVEMENT_PAGES is already viewport-based. I guess we need 4 viewport-based movement steps: move vertically by pages (GTK_MOVEMENT_PAGES) move horizontally by pages (PAGE_HORIZONTALLY_HACK_VALUE) move vertically by steps (currently missing, GTK_MOVEMENT_SCROLL_VERTICALLY ?) move horizontally by steps (currently missing, GTK_MOVEMENT_SCROLL_HORIZONTALLY ?) Committed to stable.
Here is a patch which adds a separate enum, GtkScrollStep, and a move_viewport keybinding signal.
Created attachment 13842 [details] [review] patch
Does it make sense to do this *and* add GTK_MOVEMENT_HORIZONTAL_PAGES?
I think it is cleaner to separate the cursor movement from the viewport movement, so yes I think it makes sense to do both.
So, the idea of this patch is to enable people to have the ability to hook up scroll-viewport bindings, but not to actually add any by default? I'm not particularly opposed to that, though it strikes me as something we wouldn't normally do. Need to add the signal as a binding signal ... current patch breaks bincompat. Shouldn't LINE_ENDS do HORIZONTAL_ENDS? - that is, I'd expect Home to scroll fully to the left and End fully to the right.
Created attachment 15847 [details] [review] new patch
Redone as binding signal, also mapping LINE_ENDS to HORIZONTAL_ENDS now.
Looks good to me.