Created attachment 246605 [details] [review]
Position the caret navigation cursor by clicking

Review of attachment 246605 [details] [review]:

Patch looks good in general, but we should place the cursor on button_press, the caret page needs to be updated and the cursor position is not accurate with the patch.

::: libview/ev-view.c
@@ +4341,3 @@
+{
+	EvRectangle *areas = NULL;
+	EvRectangle *rect = NULL;

You don't need to initialize this one.

@@ +4345,3 @@
+	guint i;
+
+	*offset = -1;

This value doesn't really matter if the function returns FALSE, so we don't need to intialize it.

@@ +4350,3 @@
+		return FALSE;
+
+	ev_page_cache_get_text_layout (view->page_cache, view->current_page, &areas, &n_areas);

This should be the page you got from get_doc_point_from_location() instead of current page and it should be returned as an out parameter like the offset.

@@ +4354,3 @@
+		return FALSE;
+
+	for (i = 0; i < n_areas && *offset < 0; i++) {

You can simply return TRUE when you find the offset.

@@ +4357,3 @@
+		rect = areas + i;
+		if (x >= rect->x1 && x <= rect->x2 &&
+		    y >= rect->y1 && y <= rect->y2)

The problem with this is that you are checking if the point is inside the character bounding box, but in most of the cases character bounding boxes overlap, so you would be positioning for cursor for the previous character of the one you clicked over. A possible approach would be to divide the character bounding box in two halves, if the point is in the first half, offset is the current character (i) and if it's inside the second half the offset is the next character (i + 1).

@@ +4453,3 @@
 			ev_view_handle_link (view, link);
 		}
+	} else if (view->caret_enabled && view->rotation == 0) {

This should be done in button_press, not in release.

@@ +4457,3 @@
+
+		if (get_doc_point_from_location (view, event->x, event->y, &page, &x, &y) &&
+		    get_offset_from_doc_point (view, x, y, &offset)) {

We could merge this both into a single function get_caret_cursor_offset_at_location() that receives a location in view coordinates, similar to other _at_location methods in EvView, and calls get_doc_point_from_location() for the given x, y values. If we also check if caret is enable and rotation is 0 this if would be simpler.

@@ +4458,3 @@
+		if (get_doc_point_from_location (view, event->x, event->y, &page, &x, &y) &&
+		    get_offset_from_doc_point (view, x, y, &offset)) {
+			view->cursor_offset = offset;

We should also update the cursor page, get_caret_cursor_offset_at_location() should return both the offset and the page.

Created attachment 247055 [details] [review]
Position the caret cursor by clicking

Patch updated according to comments in comment 2

Review of attachment 247055 [details] [review]:

Great, pushed to git master (with just some minor cosmetic changes). Thanks!