Closed
Bug 245931
Opened 20 years ago
Closed 20 years ago
Problems with accessibility API focus events
Categories
(Core :: Disability Access APIs, defect, P1)
Tracking
()
RESOLVED
FIXED
People
(Reporter: aaronlev, Assigned: aaronlev)
Details
(Keywords: access)
Attachments
(1 file, 1 obsolete file)
6.79 KB,
patch
|
pkwarren
:
review+
bryner
:
superreview+
|
Details | Diff | Splinter Review |
Problems: 1. When we fire focus events in the accessibility API, the accessibility object that's focused doesn't always have STATE_FOCUSED marked. 2. We're occasionally firing a focus event on the object being tabbed *from*, which is totally wrong. This bug is stopping Window-Eyes from working correctly with Mozilla.
Assignee | ||
Comment 1•20 years ago
|
||
Assignee | ||
Updated•20 years ago
|
Attachment #150306 -
Flags: superreview?(bryner)
Attachment #150306 -
Flags: review?(pkw)
Comment 2•20 years ago
|
||
Comment on attachment 150306 [details] [diff] [review] 1) Make sure STATE_FOCUSED is set on objects focus event indicates are focused, 2) Don't fire events using ExplicitOriginalTarget unless the current target has a binding parent >Index: src/base/nsRootAccessible.cpp >=================================================================== >+NS_IMETHODIMP nsRootAccessible::GetState(PRUint32 *aState) >+{ >+ nsresult rv = NS_ERROR_FAILURE; >+ if (mDOMNode) { >+ rv = nsDocAccessibleWrap::GetState(aState); >+ } >+ if (NS_FAILED(rv)) { >+ return rv; >+ } Do you mean to return an error here if mDOMNode is not set? Just wondering... >+ if (gLastFocusedNode) { >+ nsCOMPtr<nsIDOMDocument> rootAccessibleDoc(do_QueryInterface(mDocument)); >+ nsCOMPtr<nsIDOMDocument> focusedDoc; >+ gLastFocusedNode->GetOwnerDocument(getter_AddRefs(focusedDoc)); >+ if (rootAccessibleDoc == focusedDoc) { Is it possible for both rootAccessibleDoc and focusedDoc to be null and you still set STATE_FOCUSED? > gLastFocusedNode = focusNode; > NS_ADDREF(gLastFocusedNode); >+ if (role == ROLE_TEXT) { >+ printf("Hello"); >+ } This should be taken out in the final patch. >- if (!content || content->IsContentOfType(nsIContent::eHTML)) { ... >+ if (content && content->IsContentOfType(nsIContent::eHTML) && You did mean to switch these around, right? >+ // The difference between event target types is not 100% clear from Incomplete comment.
Assignee | ||
Comment 3•20 years ago
|
||
(In reply to comment #2) > (From update of attachment 150306 [details] [diff] [review]) > >Index: src/base/nsRootAccessible.cpp > >=================================================================== > >+NS_IMETHODIMP nsRootAccessible::GetState(PRUint32 *aState) > >+{ > >+ nsresult rv = NS_ERROR_FAILURE; > >+ if (mDOMNode) { > >+ rv = nsDocAccessibleWrap::GetState(aState); > >+ } > >+ if (NS_FAILED(rv)) { > >+ return rv; > >+ } > > Do you mean to return an error here if mDOMNode is not set? Just wondering... Yes I did. When mDOMNode for any kind of nsAccessible/nsAccessNode is nsnull, then it is no longer a relevant object. > > >+ if (gLastFocusedNode) { > >+ nsCOMPtr<nsIDOMDocument> rootAccessibleDoc(do_QueryInterface(mDocument)); > >+ nsCOMPtr<nsIDOMDocument> focusedDoc; > >+ gLastFocusedNode->GetOwnerDocument(getter_AddRefs(focusedDoc)); > >+ if (rootAccessibleDoc == focusedDoc) { > > Is it possible for both rootAccessibleDoc and focusedDoc to be null and you > still set STATE_FOCUSED? Not at this point, because rootAccessibleDoc would only be nsnull if mDOMNode was also nsnull. I'll add an assertion. > > > gLastFocusedNode = focusNode; > > NS_ADDREF(gLastFocusedNode); > >+ if (role == ROLE_TEXT) { > >+ printf("Hello"); > >+ } > > This should be taken out in the final patch. > > >- if (!content || content->IsContentOfType(nsIContent::eHTML)) { > ... > >+ if (content && content->IsContentOfType(nsIContent::eHTML) && > > You did mean to switch these around, right? Yes > > >+ // The difference between event target types is not 100% clear from > > Incomplete comment. >
Assignee | ||
Updated•20 years ago
|
Attachment #150306 -
Attachment is obsolete: true
Attachment #150306 -
Flags: superreview?(bryner)
Attachment #150306 -
Flags: review?(pkw)
Assignee | ||
Comment 4•20 years ago
|
||
Assignee | ||
Updated•20 years ago
|
Attachment #150319 -
Flags: superreview?(bryner)
Attachment #150319 -
Flags: review?(pkw)
Updated•20 years ago
|
Attachment #150319 -
Flags: review?(pkw) → review+
Assignee | ||
Updated•20 years ago
|
Priority: -- → P1
Comment 5•20 years ago
|
||
Comment on attachment 150319 [details] [diff] [review] Fixes pkw's comments and also fixes focused tree items to show STATE_FOCUSABLE sr=bryner
Attachment #150319 -
Flags: superreview?(bryner) → superreview+
Assignee | ||
Comment 6•20 years ago
|
||
Checking in src/base/nsAccessible.cpp; /cvsroot/mozilla/accessible/src/base/nsAccessible.cpp,v <-- nsAccessible.cpp new revision: 1.102; previous revision: 1.101 done Checking in src/base/nsRootAccessible.cpp; /cvsroot/mozilla/accessible/src/base/nsRootAccessible.cpp,v <-- nsRootAccessible.cpp new revision: 1.92; previous revision: 1.91 done Checking in src/base/nsRootAccessible.h; /cvsroot/mozilla/accessible/src/base/nsRootAccessible.h,v <-- nsRootAccessible.h new revision: 1.40; previous revision: 1.39 done Checking in src/xul/nsXULFormControlAccessible.cpp; /cvsroot/mozilla/accessible/src/xul/nsXULFormControlAccessible.cpp,v <-- nsXULFormControlAccessible.cpp new revision: 1.36; previous revision: 1.35 done
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•