Skip to content

Commit

Permalink
EvView: Fix cursor movement when logical and visual line order differs
Browse files Browse the repository at this point in the history
Make sure not to move the caret in the wrong direction when restoring
the visual line X offset, in case the visual and logical order is
slightly different.

The algorithm used to move the cursor on the next line and restore the
X position across lines works as follows:

1. Move `cursor_offset` to the next line by incrementing it until
   reaching a line break;
2. Find the Y coordinate corresponding to the new cursor_offset;
3. Find the text closest to the new Y coordinate and the previous X
   coordinate.
4. Move cursor_offset to the text at this new (X, Y) location.

The issue lies in step 3, which can find a position on a different line
than expected in case several lines have a nearly the same Y position.

Evince references:
* https://gitlab.gnome.org/GNOME/evince/issues/889
* https://gitlab.gnome.org/GNOME/evince/merge_requests/81
* https://gitlab.gnome.org/GNOME/evince/commit/dddd98b4c7922e2906bba6a31afa07837ae6c39c
  • Loading branch information
cwendling authored and raveit65 committed Oct 20, 2018
1 parent f4a7715 commit a7d0b8d
Showing 1 changed file with 10 additions and 0 deletions.
10 changes: 10 additions & 0 deletions libview/ev-view.c
Expand Up @@ -5406,6 +5406,7 @@ ev_view_move_cursor (EvView *view,
gint prev_page;
cairo_region_t *damage_region;
gboolean clear_selections = FALSE;
const gboolean forward = count >= 0;

if (!view->caret_enabled || view->rotation != 0)
return FALSE;
Expand Down Expand Up @@ -5475,9 +5476,18 @@ ev_view_move_cursor (EvView *view,
return TRUE;

if (step == GTK_MOVEMENT_DISPLAY_LINES) {
const gint prev_cursor_offset = view->cursor_offset;

position_caret_cursor_at_location (view,
MAX (rect.x, view->cursor_line_offset),
rect.y + (rect.height / 2));
/* Make sure we didn't move the cursor in the wrong direction
* in case the visual order isn't the same as the logical one,
* in order to avoid cursor movement loops */
if ((forward && prev_cursor_offset > view->cursor_offset) ||
(!forward && prev_cursor_offset < view->cursor_offset)) {
view->cursor_offset = prev_cursor_offset;
}
if (!clear_selections &&
prev_offset == view->cursor_offset && prev_page == view->cursor_page) {
gtk_widget_error_bell (GTK_WIDGET (view));
Expand Down

0 comments on commit a7d0b8d

Please sign in to comment.