relations not working when pointing to a <span>

VERIFIED FIXED

Status

()

Core
Disability Access APIs
--
major
VERIFIED FIXED
10 years ago
9 years ago

People

(Reporter: Scott Haeger, Assigned: Evan Yan)

Tracking

(Blocks: 2 bugs, {access})

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

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: orca:high)

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

10 years ago
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

Updated

10 years ago
Blocks: 343213

Updated

10 years ago
Keywords: access
(Reporter)

Comment 1

10 years ago
For http://accessibleajax.clcworld.net/simple/controls.htm I am seeing the controllerfor relation but not the controlledby relation.

Updated

10 years ago
Severity: normal → major

Comment 2

10 years ago
<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>

Updated

10 years ago
Depends on: 371156

Comment 3

10 years ago
Ok, I have a fix for this in bug 371156.

Comment 4

10 years ago
Fixed by checkin to bug 371156.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED

Comment 5

10 years ago
Had to back that one out.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Updated

10 years ago
Blocks: 374212

Updated

10 years ago
Blocks: 396346

Updated

10 years ago
Whiteboard: orca:high

Comment 6

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

Comment 7

10 years ago
Created attachment 294835 [details] [diff] [review]
create accessible when it has a relation

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)

Comment 8

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

Comment 9

10 years ago
Created attachment 295051 [details] [diff] [review]
overload FindNeighbourPointingToNode
Attachment #294835 - Attachment is obsolete: true
Attachment #295051 - Flags: review?(surkov.alexander)
Attachment #294835 - Flags: review?(surkov.alexander)
(Assignee)

Comment 10

10 years ago
Created attachment 295053 [details] [diff] [review]
also check aria_activedescendant
Attachment #295051 - Attachment is obsolete: true
Attachment #295053 - Flags: review?(surkov.alexander)
Attachment #295051 - Flags: review?(surkov.alexander)

Comment 11

10 years ago
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.

Updated

10 years ago
Attachment #295053 - Flags: review?(surkov.alexander) → review-
(Assignee)

Comment 12

10 years ago
(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?

Comment 13

10 years ago
(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 14

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

Re-requesting review.
Attachment #295053 - Flags: review- → review?(surkov.alexander)
(Assignee)

Updated

10 years ago
Attachment #295053 - Flags: review?(aaronleventhal)

Comment 15

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

Comment 16

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

Comment 17

10 years ago
Created attachment 297128 [details] [diff] [review]
addression Aaron's comment
Attachment #295053 - Attachment is obsolete: true
Attachment #297128 - Flags: review?(aaronleventhal)
Attachment #295053 - Flags: review?(surkov.alexander)
Attachment #295053 - Flags: review?(aaronleventhal)

Comment 18

10 years ago
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)

Comment 19

10 years ago
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?

Comment 20

10 years ago
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 21

10 years ago
Comment on attachment 297128 [details] [diff] [review]
addression Aaron's comment

looks ok, r=me
Attachment #297128 - Flags: review?(surkov.alexander) → review+

Updated

10 years ago
Attachment #297128 - Flags: approval1.9?

Updated

10 years ago
Attachment #297128 - Flags: approval1.9? → approval1.9+
(Assignee)

Comment 22

10 years ago
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
Last Resolved: 10 years ago10 years ago
Resolution: --- → FIXED

Updated

10 years ago
No longer depends on: 371156
(Reporter)

Updated

10 years ago
Status: RESOLVED → VERIFIED

Comment 23

9 years ago
in-testsuite?
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.