Closed
Bug 761102
Opened 12 years ago
Closed 12 years ago
focus may be missed when ARIA active-descendant is changed on active composite widget
Categories
(Core :: Disability Access APIs, defect)
Core
Disability Access APIs
Tracking
()
RESOLVED
FIXED
mozilla18
People
(Reporter: surkov, Assigned: surkov)
References
(Blocks 1 open bug)
Details
(Keywords: access)
Attachments
(1 file)
5.10 KB,
patch
|
tbsaunde
:
review+
davidb
:
feedback+
|
Details | Diff | Splinter Review |
Assignee | ||
Comment 1•12 years ago
|
||
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #629732 -
Flags: review?(trev.saunders)
Attachment #629732 -
Flags: feedback?(dbolter)
Updated•12 years ago
|
Attachment #629732 -
Flags: feedback?(dbolter) → feedback+
Comment 2•12 years ago
|
||
Comment on attachment 629732 [details] [diff] [review] patch >+ if (mRoleMapEntry && mRoleMapEntry->Is(nsGkAtoms::combobox)) { I'm not really sure why that seems better than checking mRolemapEntry->role, but it seems fine. >+ PRUint32 childCount = ChildCount(); >+ for (PRUint32 idx = 0; idx < childCount; idx++) { >+ Accessible* child = mChildren.ElementAt(idx); >+ if (child->Role() == roles::ENTRY) >+ return FocusMgr()->HasDOMFocus(child->GetContent()); I'd say this is fine, but couldn't somebody do something crazy like <div role="combobox"> <div><input></div> <ul> <li>blah</li> </ul> </div> for which we'd need to search the subtree nsGkAtoms::aria_activedescendant, id)) { >- nsIDocument* DOMDoc = aElm->OwnerDoc(); >- dom::Element* activeDescendantElm = DOMDoc->GetElementById(id); >+ dom::Element* activeDescendantElm = aElm->OwnerDoc()->GetElementById(id); so it doesn't actually have to be a descendant? or is this just faster because the doc has a fast mapping of id -> elem? /test_focus_aria_activedescendant.html >+++ b/accessible/tests/mochitest/events/test_focus_aria_activedescendant.html >@@ -65,16 +65,20 @@ https://bugzilla.mozilla.org/show_bug.cg > > var gQueue = null; > function doTest() > { > gQueue = new eventQueue(); > > gQueue.push(new synthFocus("container", new focusChecker("item1"))); > gQueue.push(new changeARIAActiveDescendant("container", "item2")); >+ >+ gQueue.push(new synthFocus("combobox_entry", new focusChecker("combobox_entry"))); >+ gQueue.push(new changeARIAActiveDescendant("combobox", "combobox_option2")); >+ > todo(false, "No focus for inserted element, bug 687011"); > //gQueue.push(new insertItemNFocus("container", "item3")); > > gQueue.invoke(); // Will call SimpleTest.finish(); > } > > SimpleTest.waitForExplicitFinish(); > addA11yLoadEvent(doTest); >@@ -82,19 +86,32 @@ https://bugzilla.mozilla.org/show_bug.cg > </head> > <body> > > <a target="_blank" > href="https://bugzilla.mozilla.org/show_bug.cgi?id=429547" > title="Support aria-activedescendant usage in nsIAccesible::TakeFocus()"> > Mozilla Bug 429547 > </a> >+ <a target="_blank" >+ href="https://bugzilla.mozilla.org/show_bug.cgi?id=761102" >+ title="Focus may be missed when ARIA active-descendant is changed on active composite widget"> >+ Mozilla Bug 761102 >+ </a> > <p id="display"></p> > <div id="content" style="display: none"></div> > <pre id="test"> > </pre> > > <div role="listbox" aria-activedescendant="item1" id="container" tabindex="1"> > <div role="listitem" id="item1">item1</div> > <div role="listitem" id="item2">item2</div> > </div> >+ >+ <div role="combobox" id="combobox"> >+ <input id="combobox_entry"> >+ <ul> >+ <li role="option" id="combobox_option1">option1</li> >+ <li role="option" id="combobox_option2">option2</li> >+ </ul> >+ </div> > </body> > </html>
Attachment #629732 -
Flags: review?(trev.saunders) → review+
Assignee | ||
Comment 3•12 years ago
|
||
(In reply to Trevor Saunders (:tbsaunde) from comment #2) > >+ if (mRoleMapEntry && mRoleMapEntry->Is(nsGkAtoms::combobox)) { > > I'm not really sure why that seems better than checking mRolemapEntry->role, > but it seems fine. a bit faster, constants instead strings > >+ PRUint32 childCount = ChildCount(); > >+ for (PRUint32 idx = 0; idx < childCount; idx++) { > >+ Accessible* child = mChildren.ElementAt(idx); > >+ if (child->Role() == roles::ENTRY) > >+ return FocusMgr()->HasDOMFocus(child->GetContent()); > > I'd say this is fine, but couldn't somebody do something crazy like > > <div role="combobox"> > <div><input></div> > <ul> > <li>blah</li> > </ul> > </div> > > for which we'd need to search the subtree the author can do anything, the spec says that example is possible but doesn't provide the exact rules. If somebody has a reason to do deep trees then we can fix it. > nsGkAtoms::aria_activedescendant, id)) { > >- nsIDocument* DOMDoc = aElm->OwnerDoc(); > >- dom::Element* activeDescendantElm = DOMDoc->GetElementById(id); > >+ dom::Element* activeDescendantElm = aElm->OwnerDoc()->GetElementById(id); > > so it doesn't actually have to be a descendant? or is this just faster > because the doc has a fast mapping of id -> elem? iirc I filed bug about this but on the another hand the spec allows this iirc.
Assignee | ||
Comment 4•12 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/4168b071661e
Flags: in-testsuite+
Target Milestone: --- → mozilla18
Comment 5•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/4168b071661e
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 6•12 years ago
|
||
(In reply to alexander :surkov from comment #3) > (In reply to Trevor Saunders (:tbsaunde) from comment #2) > > > >+ if (mRoleMapEntry && mRoleMapEntry->Is(nsGkAtoms::combobox)) { > > > > I'm not really sure why that seems better than checking mRolemapEntry->role, > > but it seems fine. > > a bit faster, constants instead strings I meant the a11y::Role which I believe would actually be epsilon faster since it would be a compare of 2 dwords not 2 qwords.
Assignee | ||
Comment 7•12 years ago
|
||
(In reply to Trevor Saunders (:tbsaunde) from comment #6) > (In reply to alexander :surkov from comment #3) > > (In reply to Trevor Saunders (:tbsaunde) from comment #2) > > > > > >+ if (mRoleMapEntry && mRoleMapEntry->Is(nsGkAtoms::combobox)) { > > > > > > I'm not really sure why that seems better than checking mRolemapEntry->role, > > > but it seems fine. > > > > a bit faster, constants instead strings > > I meant the a11y::Role which I believe would actually be epsilon faster > since it would be a compare of 2 dwords not 2 qwords. ah, I see, I guess I wanted to operate on ARIA roles since in general ARIA role might be not 1 to 1 equivalence to accessible role. I don't have strong opinion on this though.
You need to log in
before you can comment on or make changes to this bug.
Description
•