diff --git a/CHANGELOG.md b/CHANGELOG.md index 49fb6b5a..e8f0ca7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,8 @@ when the mouse button is released - not as soon as `shift` is released. * Selected cells did not appear selected if modified. +* Very rare crash when beginning a selection at the same time the + terminal content was scrolled. ### Security diff --git a/selection.c b/selection.c index 9b3b8ffd..69acbe4e 100644 --- a/selection.c +++ b/selection.c @@ -30,31 +30,39 @@ selection_enabled(const struct terminal *term) } bool -selection_on_rows_in_view(const struct terminal *term, int row_start, int row_end) +selection_on_rows(const struct terminal *term, int row_start, int row_end) { - LOG_DBG("selection: %d-%d, range: %d-%d (view=%d)", + LOG_DBG("on rows: %d-%d, range: %d-%d (offset=%d)", term->selection.start.row, term->selection.end.row, - row_start, row_end, term->grid->view); + row_start, row_end, term->grid->offset); - if (term->selection.start.row == -1 || term->selection.end.row == -1) + if (term->selection.end.row == -1) return false; + assert(term->selection.start.row != -1); + + row_start += term->grid->offset; + row_end += term->grid->offset; + const struct coord *start = &term->selection.start; const struct coord *end = &term->selection.end; + if ((row_start <= start->row && row_end >= start->row) || + (row_start <= end->row && row_end >= end->row)) + { + /* The range crosses one of the selection boundaries */ + return true; + } + + /* For the last check we must ensure start <= end */ if (start->row > end->row) { const struct coord *tmp = start; start = end; end = tmp; } - row_start += term->grid->view; - row_end += term->grid->view; - - if ((row_start <= start->row && row_end >= start->row) || - (row_start <= end->row && row_end >= end->row) || - (row_start >= start->row && row_end <= end->row)) - { + if (row_start >= start->row && row_end <= end->row) { + LOG_INFO("ON ROWS"); return true; } diff --git a/selection.h b/selection.h index 87d25fc5..3a520820 100644 --- a/selection.h +++ b/selection.h @@ -17,7 +17,7 @@ void selection_dirty_cells(struct terminal *term); void selection_cancel(struct terminal *term); void selection_extend(struct terminal *term, int col, int row, uint32_t serial); -bool selection_on_rows_in_view(const struct terminal *term, int start, int end); +bool selection_on_rows(const struct terminal *term, int start, int end); void selection_mark_word(struct terminal *term, int col, int row, bool spaces_only, uint32_t serial); diff --git a/terminal.c b/terminal.c index a4a857a5..de8d7146 100644 --- a/terminal.c +++ b/terminal.c @@ -1722,6 +1722,22 @@ term_cursor_blink_restart(struct terminal *term) } } +static bool +selection_on_top_region(const struct terminal *term, + struct scroll_region region) +{ + return region.start > 0 && + selection_on_rows(term, 0, region.start - 1); +} + +static bool +selection_on_bottom_region(const struct terminal *term, + struct scroll_region region) +{ + return region.end < term->rows && + selection_on_rows(term, region.end, term->rows - 1); +} + void term_scroll_partial(struct terminal *term, struct scroll_region region, int rows) { @@ -1731,8 +1747,32 @@ term_scroll_partial(struct terminal *term, struct scroll_region region, int rows /* Clamp scroll amount */ rows = min(rows, region.end - region.start); - if (selection_on_rows_in_view(term, region.end - rows, region.end)) - selection_cancel(term); + /* Cancel selections that cannot be scrolled */ + if (term->selection.start.row != -1) { + if (term->selection.end.row != -1) { + /* + * Selection is (partly) inside either the top or bottom + * scrolling regions, or on (at least one) of the lines + * scrolled in (i.e. re-used lines). + */ + if (selection_on_top_region(term, region) || + selection_on_bottom_region(term, region) || + selection_on_rows(term, region.end - rows, region.end - 1)) + { + selection_cancel(term); + } + } else { + /* + * User started a selection, but didn't move the + * cursor. + * + * Not 100% sure this is needed for forward scrolling, but + * let's keep it here, for consistency with reverse + * scrolling. + */ + selection_cancel(term); + } + } bool view_follows = term->grid->view == term->grid->offset; term->grid->offset += rows; @@ -1779,9 +1819,31 @@ term_scroll_reverse_partial(struct terminal *term, /* Clamp scroll amount */ rows = min(rows, region.end - region.start); - /* Does the selection cover re-used, newly scrolled in lines? */ - if (selection_on_rows_in_view(term, region.start, region.start + rows - 1)) - selection_cancel(term); + /* Cancel selections that cannot be scrolled */ + if (term->selection.start.row != -1) { + if (term->selection.end.row != -1) { + /* + * Selection is (partly) inside either the top or bottom + * scrolling regions, or on (at least one) of the lines + * scrolled in (i.e. re-used lines). + */ + if (selection_on_top_region(term, region) || + selection_on_bottom_region(term, region) || + selection_on_rows(term, region.start, region.start + rows - 1)) + { + selection_cancel(term); + } + } else { + /* + * User started a selection, but didn't move the + * cursor. + * + * Since we're scrolling in reverse, the result may be a + * *huge* selection that covers empty (NULL) lines. + */ + selection_cancel(term); + } + } bool view_follows = term->grid->view == term->grid->offset; term->grid->offset -= rows;