414133 - IAccessibleText nSelections and Selection methods not working

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Michael Curran]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b3pre) Gecko/2008012504 Minefield/3.0b3pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b3pre) Gecko/2008012504 Minefield/3.0b3pre

It seems that with both NVDA and Accessibility Probe, if text is selected with in an editable text control, IAccessibleText::nSelections gives 0, and IAccessibleText::Selection (given an argument of 0) causes a COM exception of 'unspecified error'. The caretOffset works perfectly, its just when the selection is not collapsed to a single point, its impossible to retreave the selection information.

Reproducible: Always

Steps to Reproduce:
1. Move to the location bar of Firefox 3.
2. select the text that is there (type some in if there's no text)
3. Open Accessibility Probe and find the location bar of Firefox 3
4. In the Accessibility properties view, find the AccessibleText node and expand that, noticing the value of SelectionCount
Actual Results:  
selectionCount is 0

Expected Results:  
selectionCount should be 1

Current trunk snapshots of NVDA from www.nvda-project.org/snapshots could also be tried, when selecting text in any Gecko 1.9 edit field NVDA does not announce the selection, though it does in other applications.

Comment 1

[alexander :surkov (:asurkov)]

Possibly the problem is we call selection2->GetRangesForIntervalCOMArray with html:input nodes though selection is somewhere inside of html:input document, so even selection holds one range but that method returns 0.

Comment 2

[Aaron Leventhal]

That sounds exactly right. 

There is code in nsHyperTextAccessible::CacheChildren() for getting the editor content root node.

http://mxr.mozilla.org/seamonkey/source/accessible/src/html/nsHyperTextAccessible.cpp#219

Comment 3

[Aaron Leventhal]

Actually that is supposed to be handled by // Case 1: plain text editor in GetSelections():
http://mxr.mozilla.org/seamonkey/source/accessible/src/html/nsHyperTextAccessible.cpp#1600

Comment 4

[Aaron Leventhal]

Attachment: When getting selection ranges, use start content node from editor if available

Comment 5

[Aaron Leventhal]

Mick, here are test try server builds with this change. Can you try it?
https://build.mozilla.org/tryserver-builds/2008-01-27_13:[e-mail]-TextfieldAccessibleSelection414133/

Comment 6

[Michael Curran]

(In reply to comment #5)
> Mick, here are test try server builds with this change. Can you try it?
> https://build.mozilla.org/tryserver-builds/2008-01-27_13:[e-mail]-TextfieldAccessibleSelection414133/
The test build works great. I have tested in the location bar and in some edit fields  with in web pages. Selections seem to work as expected now.

Comment 7

[alexander :surkov (:asurkov)]

Comment on attachment 299614 [details] [diff] [review]
When getting selection ranges, use start content node from editor if available

>Index: accessible/src/html/nsHyperTextAccessible.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/accessible/src/html/nsHyperTextAccessible.cpp,v
>retrieving revision 1.105
>diff -p -u -5 -r1.105 nsHyperTextAccessible.cpp
>--- accessible/src/html/nsHyperTextAccessible.cpp	25 Jan 2008 13:47:48 -0000	1.105
>+++ accessible/src/html/nsHyperTextAccessible.cpp	27 Jan 2008 21:48:16 -0000
>@@ -1606,11 +1606,11 @@ nsresult nsHyperTextAccessible::GetSelec
>       NS_ENSURE_TRUE(*aSelCon, NS_ERROR_FAILURE);
>     }
> 
>     editor->GetSelection(getter_AddRefs(domSel));
>     NS_ENSURE_TRUE(domSel, NS_ERROR_FAILURE);
>-    }
>+  }
>   else {
>     // Case 2: rich content subtree (can be rich editor)
>     // This uses the selection controller from the entire document
>     nsIFrame *frame = GetFrame();
>     NS_ENSURE_TRUE(frame, NS_ERROR_FAILURE);
>@@ -1632,18 +1632,29 @@ nsresult nsHyperTextAccessible::GetSelec
>   }
>   if (aRanges) {
>     nsCOMPtr<nsISelection2> selection2(do_QueryInterface(domSel));
>     NS_ENSURE_TRUE(selection2, NS_ERROR_FAILURE);
> 
>+    nsCOMPtr<nsIDOMNode> startNode;
>+    if (editor) {
>+      nsCOMPtr<nsIDOMElement> editorRoot;
>+      editor->GetRootElement(getter_AddRefs(editorRoot));
>+      startNode = do_QueryInterface(editorRoot);
>+    }

it sounds it doesn't correct for accessibles inside html editor for example, for those accessibles that are contained by editor. So if we have div inside editable document then we should try div node rather than root of editor.

>+    else
>+      startNode = mDOMNode;
>+
>+    NS_ENSURE_STATE(startNode);
>+
>     nsCOMPtr<nsIDOMNodeList> childNodes;
>-    nsresult rv = mDOMNode->GetChildNodes(getter_AddRefs(childNodes));
>+    nsresult rv = startNode->GetChildNodes(getter_AddRefs(childNodes));
>     NS_ENSURE_SUCCESS(rv, rv);
>     PRUint32 numChildren; 
>     rv = childNodes->GetLength(&numChildren);
>     NS_ENSURE_SUCCESS(rv, rv);
>-    rv = selection2->GetRangesForIntervalCOMArray(mDOMNode, 0,
>-                                                  mDOMNode, numChildren,
>+    rv = selection2->GetRangesForIntervalCOMArray(startNode, 0,
>+                                                  startNode, numChildren,
>                                                   PR_TRUE, aRanges);
>     NS_ENSURE_SUCCESS(rv, rv);
>     // Remove collapsed ranges
>     PRInt32 numRanges = aRanges->Count();
>     for (PRInt32 count = 0; count < numRanges; count ++) {

Comment 8

[alexander :surkov (:asurkov)]

possibly it would be enough to check peditor there if plain text editor can't have text accessibles inside itself othrewise we should provide additional method to know whether accessible owns the editor or editor owns the accessible.

Comment 9

[Aaron Leventhal]

Attachment: Just use editor root for plain text editors

Comment 10

[alexander :surkov (:asurkov)]

Comment on attachment 299664 [details] [diff] [review]
Just use editor root for plain text editors

this sounds correct, r=me

Comment 11

[Damon Sicore (:damons)]

Comment on attachment 299664 [details] [diff] [review]
Just use editor root for plain text editors

a1.9+=damons