relations not working when pointing to a <span>

VERIFIED FIXED

Status

()

defect
--
major
VERIFIED FIXED
12 years ago
11 years ago

People

(Reporter: scott, Assigned: evan.yan)

Tracking

(Blocks 2 bugs, {access})

Trunk
x86
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: orca:high)

Attachments

(1 attachment, 3 obsolete attachments)

Live region areas are not providing any relations.  I would expect to see the following relations on these test cases.

Test cases:
labelledby: http://accessibleajax.clcworld.net/simple/labelledby.htm
describedby: http://accessibleajax.clcworld.net/simple/describedby.htm

I also remember seeing labelledby relations for Real v Nicks, but there is no aria markup.  Is the XHTML attribute 'header' responsible for setting the relation?

Referring bug is #392061
Blocks: aria
Keywords: access
For http://accessibleajax.clcworld.net/simple/controls.htm I am seeing the controllerfor relation but not the controlledby relation.
Severity: normal → major
<span>s don't have accessible objects, so there is nothing to point to. 

Workaround:
Use a role on the <span>, or use a tag that always gets an accessible, e.g. <div>

Long term solution:
Fix bug 381599

We nee to have Charles Chen fix his examples using the workarounds for now.
Depends on: 381599
Summary: Live region areas are not providing labelledby relations → relations not working when pointing to a <span>
Depends on: 371156
Ok, I have a fix for this in bug 371156.
Fixed by checkin to bug 371156.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Had to back that one out.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Blocks: orca
Blocks: fox3access
Whiteboard: orca:high
Instead of exposing *every* object with an ID, we should fix this by making sure we expose an object with an ID only if it has some relation pointing to it.
Also made nsAccUtils::FindNeighbourPointingToNode() methods accept multiple relation attributes, so that we can improve performance.
Assignee: aaronleventhal → Evan.Yan
Status: REOPENED → ASSIGNED
Attachment #294835 - Flags: review?(surkov.alexander)
Evan, what do you think about to have two methods FindNeighbourPointingToNode, one of them with existing syntax, another one should take in array and its length.

Like:

FindNeighbourPointingToNode(nsIAtom *aAtom)
{
  FindNeighbourPointingToNode(&aAtom, 1)
}
Attachment #294835 - Attachment is obsolete: true
Attachment #295051 - Flags: review?(surkov.alexander)
Attachment #294835 - Flags: review?(surkov.alexander)
Attachment #295051 - Attachment is obsolete: true
Attachment #295053 - Flags: review?(surkov.alexander)
Attachment #295051 - Flags: review?(surkov.alexander)
This is potentially a huge performance hit, since for node in the tree we are walking most of the tree several times!

Entire tree * entire tree * n relations    ... And remember each time you GetAttr() it has to go through the attributes for the node one at a time -- that's O(n). 

We should not be doing any work unless the current node has an ID, otherwise it's impossible for it to be pointed to.

The activedescendant check looks good. It will make sure that even if the current item is not the active one it still gets an accessible.

I would not bother with this one:
+                              nsAccessibilityAtoms::control, // for xul
That's only used to associate a label with a XUL form control, and all XUL form controls already have accessibles.
Attachment #295053 - Flags: review?(surkov.alexander) → review-
(In reply to comment #11)
> Entire tree * entire tree * n relations
> 
So I overloaded FindNeighbourPointingToNode to handle all relations at one time.

> We should not be doing any work unless the current node has an ID, otherwise
> it's impossible for it to be pointed to.
> 
There is check for id in FindNeighbourPointingToNode.

> I would not bother with this one:
> +                              nsAccessibilityAtoms::control, // for xul
> That's only used to associate a label with a XUL form control, and all XUL form
> controls already have accessibles.
> 
Could a "control" relation be associated with something without accessible?

In your comment #7 of bug 371156, I thought you mean "control" should be checked, did I get you wrong?
(In reply to comment #12)
> (In reply to comment #11)
> Could a "control" relation be associated with something without accessible?
> 
> In your comment #7 of bug 371156, I thought you mean "control" should be
> checked, did I get you wrong?
You're right, I said that, but right now I don't believe we want or need this "control" check for XUL. It's for pointing at form controls which already have accessible objects.

As far as the ID check, you also have the check in HasRelatedContent(), but further down. I suggest just moving that up.
  nsAutoString id;
  if (aContent && nsAccUtils::GetID(aContent, id)) {


	





Comment on attachment 295053 [details] [diff] [review]
also check aria_activedescendant

Re-requesting review.
Attachment #295053 - Flags: review- → review?(surkov.alexander)
Attachment #295053 - Flags: review?(aaronleventhal)
> As far as the ID check, you also have the check in HasRelatedContent(), but
> further down. I suggest just moving that up.
>   nsAutoString id;
>  if (aContent && nsAccUtils::GetID(aContent, id)) 

I still want to see that check moved to the top of the method. There's no reason to build the array of relations or make any extra calls if there is no ID.
(In reply to comment #15)
> > As far as the ID check, you also have the check in HasRelatedContent(), but
> > further down. I suggest just moving that up.
> >   nsAutoString id;
> >  if (aContent && nsAccUtils::GetID(aContent, id)) 
> 
> I still want to see that check moved to the top of the method. There's no
> reason to build the array of relations or make any extra calls if there is no
> ID.
> 
Yes, I will update it.

Do you need a new patch for review?
Attachment #295053 - Attachment is obsolete: true
Attachment #297128 - Flags: review?(aaronleventhal)
Attachment #295053 - Flags: review?(surkov.alexander)
Attachment #295053 - Flags: review?(aaronleventhal)
Comment on attachment 297128 [details] [diff] [review]
addression Aaron's comment

Thanks for those changes. I want Surkov to review the rest, which I didn't look at.
Attachment #297128 - Flags: review?(aaronleventhal) → review?(surkov.alexander)
Ok. The patch looks ok but I have a question about HasRelatedContent. THere you check activedescendant property on parent and if is there is then content will be accessible. If it's correct then we will get for example in XUL that every xul:box with id is accessible. How much is this correct?
There would be an accessible for any XUL box with an id, that also has an ancestor that has an activedescendant property.

Basically that's the best we can do. We need at least some generic accessible since the activedescendant attribute could point to that xul:box and thus we would need to fire a focus event for it.
Comment on attachment 297128 [details] [diff] [review]
addression Aaron's comment

looks ok, r=me
Attachment #297128 - Flags: review?(surkov.alexander) → review+
Attachment #297128 - Flags: approval1.9?
Attachment #297128 - Flags: approval1.9? → approval1.9+
Checking in nsAccessibilityService.cpp;
/cvsroot/mozilla/accessible/src/base/nsAccessibilityService.cpp,v  <--  nsAccessibilityService.cpp
new revision: 1.260; previous revision: 1.259
done
Checking in nsAccessibilityUtils.h;
/cvsroot/mozilla/accessible/src/base/nsAccessibilityUtils.h,v  <--  nsAccessibilityUtils.h
new revision: 1.19; previous revision: 1.18
done
Checking in nsAccessibilityUtils.cpp;
/cvsroot/mozilla/accessible/src/base/nsAccessibilityUtils.cpp,v  <--  nsAccessibilityUtils.cpp
new revision: 1.24; previous revision: 1.23
done
Status: ASSIGNED → RESOLVED
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
No longer depends on: 371156
Status: RESOLVED → VERIFIED
in-testsuite?
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.