After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 701746 - Enable getting and setting the caret via AtkText
Enable getting and setting the caret via AtkText
Status: RESOLVED FIXED
Product: evince
Classification: Core
Component: PDF
git master
Other Linux
: Normal normal
: ---
Assigned To: Joanmarie Diggs (IRC: joanie)
Evince Maintainers
Depends on: 638905 702078
Blocks: 677348
 
 
Reported: 2013-06-06 15:12 UTC by Joanmarie Diggs (IRC: joanie)
Modified: 2014-02-27 11:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed patch (2.09 KB, patch)
2013-06-10 09:35 UTC, Joanmarie Diggs (IRC: joanie)
needs-work Details | Review
Emits cursor-moved on ev_view_set_caret_cursor_position (886 bytes, patch)
2014-02-25 11:54 UTC, Alejandro Piñeiro Iglesias (IRC: infapi00)
committed Details | Review
Implement atk_text_set_caret_offset based on ev_view_set_caret_cursor_position (1.33 KB, patch)
2014-02-25 11:55 UTC, Alejandro Piñeiro Iglesias (IRC: infapi00)
committed Details | Review
a11y: cleaning implementation for atk_text_get_caret_offset (1.52 KB, patch)
2014-02-25 11:58 UTC, Alejandro Piñeiro Iglesias (IRC: infapi00)
committed Details | Review

Description Joanmarie Diggs (IRC: joanie) 2013-06-06 15:12:46 UTC
Orca's structural navigation makes it possible to navigate amongst items by type, e.g. headings, lists, tables, etc. This functionality depends not only upon these objects being discrete and having the correct AtkRole, but upon ATs being able to (re)position the caret within those objects via AtkText.

(Only making this a depends-on for bug 638905; while the Orca functionality also depends on bug 701720, fixing this bug does not.)
Comment 1 Joanmarie Diggs (IRC: joanie) 2013-06-10 09:35:25 UTC
Created attachment 246390 [details] [review]
proposed patch

* Use the EvView's cursor_offset rather than the GtkTextBuffer
* Return -1 rather than 0 when the offset cannot be obtained
Comment 2 Carlos Garcia Campos 2013-06-10 16:14:41 UTC
Review of attachment 246390 [details] [review]:

This might depend on bug #701732

::: libview/ev-view-accessible.c
@@ +320,1 @@
+	return view->cursor_offset;

I think we could add a private function to get the cursor that checks if caret navigation is enabled and returns the current offset or -1.

@@ +337,3 @@
 
+	view->cursor_offset = offset;
+	gtk_widget_queue_draw (widget);

Same here, this could be the function I commented in the other bug, that sets the new offset, and checks whether it has actually changed or not to emit the notify singal and queue a draw.
Comment 3 Alejandro Piñeiro Iglesias (IRC: infapi00) 2014-02-25 11:54:37 UTC
Created attachment 270258 [details] [review]
Emits cursor-moved on ev_view_set_caret_cursor_position

(In reply to comment #2)

> Same here, this could be the function I commented in the other bug, that sets
> the new offset, and checks whether it has actually changed or not to emit the
> notify singal and queue a draw.

I saw that you added that method on commit b2597d9be5a95b37f2133bf97627d9a36f023bcb

What is missing on this method is the emission of CURSOR_MOVED.
Comment 4 Alejandro Piñeiro Iglesias (IRC: infapi00) 2014-02-25 11:55:15 UTC
Created attachment 270259 [details] [review]
Implement atk_text_set_caret_offset based on ev_view_set_caret_cursor_position
Comment 5 Alejandro Piñeiro Iglesias (IRC: infapi00) 2014-02-25 11:58:43 UTC
Created attachment 270260 [details] [review]
a11y: cleaning implementation for atk_text_get_caret_offset

(In reply to comment #2)
> Review of attachment 246390 [details] [review]:
> 
> This might depend on bug #701732
> 
> ::: libview/ev-view-accessible.c
> @@ +320,1 @@
> +    return view->cursor_offset;
> 
> I think we could add a private function to get the cursor that checks if caret
> navigation is enabled and returns the current offset or -1.

I don't think that that makes sense, as that behaviour is really tied to the accessibility needs. In the same way that ev_view_set_caret_cursor_position is setting cursor_offset and cursor_page not matters if the caret is enabled, a equivalen ev_view_get_cursor_position should return the values assigned.

So this patch do basically what Joanmarie Diggs was doing, but updating due last changes.