Summary: | [GTK] Implement support for get_selection and get_n_selections | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Joanmarie Diggs <jdiggs> | ||||||||
Component: | Accessibility | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | VERIFIED FIXED | ||||||||||
Severity: | Normal | CC: | apinheiro, walker.willie, xan.lopez | ||||||||
Priority: | P2 | Keywords: | Gtk | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | Linux | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 25531 | ||||||||||
Attachments: |
|
Description
Joanmarie Diggs
2009-05-06 23:41:46 PDT
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! Created attachment 31132 [details]
getselection.patch
Implement get_selection.
Comment on attachment 31132 [details]
getselection.patch
Ok, makes sense.
We are still missing the get_n_selections part to close this bug, reopening. Comment on attachment 31132 [details]
getselection.patch
Clearing review flag.
Created attachment 31140 [details]
getnselections.patch
And get_n_selections.
Comment on attachment 31140 [details]
getnselections.patch
r=me
(In reply to comment #8) > (From update of attachment 31140 [details] [review]) > r=me > Landed as r44592, thanks! 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. 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.
(In reply to comment #11) > This workaround seems to fix > things. Confirmed. Thanks! 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. Thanks, landed with the modifications suggested as r44622. Closing as FIXED. Verifying. Thanks! |