Closed
Bug 761102
Opened 13 years ago
Closed 13 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•13 years ago
|
||
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #629732 -
Flags: review?(trev.saunders)
Attachment #629732 -
Flags: feedback?(dbolter)
|
||
Updated•13 years ago
|
Attachment #629732 -
Flags: feedback?(dbolter) → feedback+
|
||
Comment 2•13 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•13 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•13 years ago
|
||
Flags: in-testsuite+
Target Milestone: --- → mozilla18
|
||
Comment 5•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
|
|
||
Comment 6•13 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•13 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
•