Bug 25609 – [GTK] Implement support for get_selection and get_n_selections

• Component: Accessibility
• Status: VERIFIED FIXED

[Joanmarie Diggs]

The accessible text interface includes a method to get a specified selection. For more information please see http://library.gnome.org/devel/atk/unstable/AtkText.html#atk-text-get-selection.

Usage/need example: ATs are now aware that text has been selected/unselected thanks to the newly-implemented text-selection-changed events. A screen reader such as Orca should in turn present the text that just became selected/unselected. In order to do that, it needs to know the start and end offset of the selected text (which it can get via get_selection).

(Note that the other selection-related methods of the accessible text interface should be implemented too. However, get_selection is the most critical of the bunch because without it, we cannot do anything in response to a text-selection-changed event.)

[Joanmarie Diggs]

I'm going to add my request for support for get_n_selections to this bug (see http://library.gnome.org/devel/atk/unstable/AtkText.html#atk-text-get-n-selections for more info).

Rationale:

1. It's more closely related to this bug than the bug I'm going to open up for the remaining selection-related methods which need to be implemented.

2. I'm assuming it won't take much (more) work to implement, so why open a separate bug for it? :-)

If I'm wrong w.r.t. #2 or you would prefer a separate bug be opened for this, please let me know. 

Thanks!

[Xan Lopez]

Created attachment 31132 [details]
getselection.patch

Implement get_selection.

[Gustavo Noronha (kov)]

Comment on attachment 31132 [details]
getselection.patch

Ok, makes sense.

[Brent Fulgham]

Landed in @r44567.

[Xan Lopez]

We are still missing the get_n_selections part to close this bug, reopening.

[Xan Lopez]

Comment on attachment 31132 [details]
getselection.patch

Clearing review flag.

[Xan Lopez]

Created attachment 31140 [details]
getnselections.patch

And get_n_selections.

[Gustavo Noronha (kov)]

Comment on attachment 31140 [details]
getnselections.patch

r=me

[Xan Lopez]

(In reply to comment #8)
> (From update of attachment 31140 [details] [review])
> r=me
> 

Landed as r44592, thanks!

[Joanmarie Diggs]

I'm afraid I'm reopening this one. In testing I'm seeing the following unexpected/undesirable behavior:

* get_n_selections returns 1 even when no text is selected.

* get_selection returns the offsets of the selected text on the page; NOT the selected text from the current accessible.

For instance:

1. View the page for this bug.

2. Select "Kit" in "WebKit Bugzilla"

3. In Accerciser, select the accessible text object associated with, WebKit Bugzilla. In the iPython console, try:

acc.queryText().getNSelections()
  result: 1 (as expected)
acc.queryText().getSelection(0)
  result: (3, 6) (as expected)

4. In Accerciser, choose any other text object (e.g. « Prev) and repeat the above commands.

The expected result is that getNSelections would return 0 because *in the newly-chosen accessible*, no text is selected. Similarly, getSelection(0) should not indicate that text is selected.

The actual result is 1 and (3, 6) respectively.

5. Deselect the text in WebKit Bugzilla, leaving the caret between the "b" and the "K". Choose any text object and repeat the above commands.

Expected results: getNSelections would return 0 because no text is selected.

Actual results: getNSelections continues to return 1.

In addition, the offsets returned by getSelection(0) indicate the current caret position (3, 3) even if the accessible being examined doesn't have the caret.

[Xan Lopez]

Created attachment 31153 [details]
getselectionfix.patch

You are totally right! The API managed to confuse me, because object->selection() actually return the global selection, wherever it is, not the selection for the object you are using. This workaround seems to fix things.

[Joanmarie Diggs]

(In reply to comment #11)

> This workaround seems to fix
> things.

Confirmed. Thanks! 

[Gustavo Noronha (kov)]

Comment on attachment 31153 [details]
getselectionfix.patch

>         This is pretty hacky-ish, but I can't seem to find a direct API to
>         get the VisibleSelection for a given object, only the global one.

This should go as a comment in the code (as well?), IMO.
> +
> +    if (selection_num != 0 || !selectionBelongsToObject(coreObject, selection)) {
>          // WebCore does not support multiple selection, so anything but 0 does not make sense for now.

Probably here, extending this comment. I'd also move this comment to before the if, so that it is clear that you are talking about the whole conditional.

Other than that, despite being hacky, this looks good.

[Xan Lopez]

Thanks, landed with the modifications suggested as r44622. Closing as FIXED.

[Joanmarie Diggs]

Verifying. Thanks!