IAccessibleText nSelections and Selection methods not working

RESOLVED FIXED

Status

()

RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: mick, Assigned: aaronlev)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

11 years ago
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.
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 2

11 years ago
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
(Assignee)

Comment 3

11 years ago
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
(Assignee)

Comment 4

11 years ago
Created attachment 299614 [details] [diff] [review]
When getting selection ranges, use start content node from editor if available
Attachment #299614 - Flags: review?(surkov.alexander)
(Assignee)

Comment 5

11 years ago
Mick, here are test try server builds with this change. Can you try it?
https://build.mozilla.org/tryserver-builds/2008-01-27_13:54-aaronleventhal@moonset.net-TextfieldAccessibleSelection414133/
(Reporter)

Comment 6

11 years ago
(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:54-aaronleventhal@moonset.net-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 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 ++) {
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.
(Assignee)

Comment 9

11 years ago
Created attachment 299664 [details] [diff] [review]
Just use editor root for plain text editors
Attachment #299614 - Attachment is obsolete: true
Attachment #299664 - Flags: review?(surkov.alexander)
Attachment #299614 - Flags: review?(surkov.alexander)
Comment on attachment 299664 [details] [diff] [review]
Just use editor root for plain text editors

this sounds correct, r=me
Attachment #299664 - Flags: review?(surkov.alexander) → review+
(Assignee)

Updated

11 years ago
Attachment #299664 - Flags: approval1.9?
Comment on attachment 299664 [details] [diff] [review]
Just use editor root for plain text editors

a1.9+=damons
Attachment #299664 - Flags: approval1.9? → approval1.9+
(Assignee)

Updated

11 years ago
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.