Created attachment 246619 [details] [review]
Text selection

Created attachment 247361 [details] [review]
Text selection

Update the previous patch, as it had become obsolete because EvView is using gtkbindings now.

Review of attachment 247361 [details] [review]:

I like it a lot, but there are a few issues.

::: libview/ev-view.c
@@ +4963,3 @@
+select_rectangle (EvView *view,
+		  GdkRectangle *prev_rect,
+		  GdkRectangle *rect)

This method is a bit confusing, it's called select_rectangle but it receives two rectangles. The method is only used to extend the selection from the cursor, select_rectangle sounds too generic. I think this could be called something like extend_selection_from_cursor and it only receives an end point. The computation of the cursor point can be done here instead of in ev_view_move_cursor.

@@ +5064,3 @@
+		GdkRectangle prev_rect;
+
+		if (!get_caret_cursor_rect_from_offset (view, prev_offset, prev_page, &prev_rect))

There's a small problem with this approach. get_caret_cursor_rect_from_offset() is a bit confusing too, because it doesn't return the cursor_rect, but the bbox of the character where the cursor should be at. The method that actually returns the cursor_rect is get_caret_cursor_area. The problem of not using the actual cursor rect, is that in some cases, specially with spaces, the selection is one character beyond the actual cursor, so we need to make sure the end point we pass to compute_selections is exactly at the cursor position.

To avoid the get_caret_cursor_rect_from_offset() confusion I've just pushed a patch to git master to merge get_caret_cursor_rect_from_offset() and get_caret_cursor_area().

Created attachment 247434 [details] [review]
Text selection using the caret

Updated patch.

Review of attachment 247434 [details] [review]:

Great, I've just pushed it to git master with the two comments I mention below fixed. Thanks!

::: libview/ev-view.c
@@ +5050,3 @@
+
+	/* Select text */
+	if (extend_selection) {

Instead of returning early in extend_selection_from_cursor when selection is not supported for the document, we can check it here and save all this in such case.

@@ +5062,2 @@
+		end_point.x = rect.x;
+		end_point.y = rect.y;

In previous patch you moved the y coords to the middle of the line, it is still needed.

*** Bug 491217 has been marked as a duplicate of this bug. ***