386813 - Support table interfaces for grid/treegrid when no HTML table undeneath

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Aaron Leventhal]

It is possible to implement an ARIA grid or treegrid using CSS positioning, instead of an HTML <table> underneath.

Right now we only support the accessible table interfaces if a real <table> is actually used.

Unfortunately, this may be quite difficult to implement.

We either need to:
1) allow an nsAccessible to QI to nsIAccessibleTable (but only when the appropriate role is attached), or 
2) we need to create a new type of object when grid or treegrid is used

I think #2 is probably okay, since a table should not also be a text/hypertext/editabletext or anything else. In that case it's perhaps not too difficult to manage. However, since it is possible to do things with an HTML table, I won't call this a priority for Firefox 3.

Comment 1

[Aaron Leventhal]

This is for Mozilla 2.0 or something like that -- not for Firefox 3.

Comment 2

[David Bolter [:davidb] (NeedInfo me for attention)]

We're getting pressure for this (bumping to Major).

Comment 3

[alexander :surkov (:asurkov)]

Attachment: patch

primary methods are implemented, I prefer to save table selection methods for another bug.

Comment 4

[[e-mail]]

Comment on attachment 377363 [details] [diff] [review]
patch

In nsARIAGridCellAccessible::GetAttributesInternal I think it might be slightly neater to set colIdx = colCount; when (child == this) thus avoiding that fiddly flag logic.

Comment 5

[David Bolter [:davidb] (NeedInfo me for attention)]

Comment on attachment 377363 [details] [diff] [review]
patch

So nice to see this started. Some comments:

>+  /**
>+   * Returns columns count in the table.
>+   * XXX: not very well name property.

Change name to named here (and elsewhere).

>+  /**
>+   * Returns table accessible coontaining column headers.

containing (here and elsewhere)

>+   * XXX: not very well name property (header vs head).
>+   */
>   readonly attribute nsIAccessibleTable  rowHeader;

Not sure I understand the naming issue here. Is "columnHeader" bad too?

>+NS_IMETHODIMP
>+nsARIAGridAccessible::GetCaption(nsIAccessible **aCaption)
>+{
>+  NS_ENSURE_ARG_POINTER(aCaption);
>+  *aCaption = nsnull;
>+
>+  if (IsDefunct())
>+    return NS_ERROR_FAILURE;
>+
>+  // XXX: should be pointed by aria-describedby on grid?

We could use aria-labelled by...

>+  // XXX: it sounds ARIA hasn't a way to provide a summary.

And here we could use aria-describedby (since describedby tends to be longer).


>+
>+NS_IMETHODIMP
>+nsARIAGridAccessible::GetRowHeader(nsIAccessibleTable **aRowHeader)
>+{
>+  NS_ENSURE_ARG_POINTER(aRowHeader);
>+  *aRowHeader = nsnull;
>+
>+  if (IsDefunct())
>+    return NS_ERROR_FAILURE;
>+
>+  // XXX: what should we return here?

I think the assumption is there is one row header per table. We should just believe the author if they have a role="rowheader" and return the accessible for that. Return the first one we find and bail -- next patch ;).


>+NS_IMETHODIMP
>+nsARIAGridAccessible::GetColumnAtIndex(PRInt32 aIndex, PRInt32 *aColumn)
>+{
>+  NS_ENSURE_ARG_POINTER(aColumn);
>+  *aColumn = -1;
>+
>+  if (IsDefunct())
>+    return NS_ERROR_FAILURE;
>+
>+  NS_ENSURE_ARG(aIndex >= 0);
>+
>+  PRInt32 rowCount = 0;
>+  GetColumns(&rowCount);

GetRows right?  (Should probably add more tests)

>+  // XXX: should we rely on aria-selected or DOM selection?

To answer all these comments (for next patch) always let aria-* override native.

>+NS_IMETHODIMP
>+nsARIAGridAccessible::IsProbablyForLayout(PRBool *aIsProbablyForLayout)
>+{
>+  NS_ENSURE_ARG_POINTER(aIsProbablyForLayout);
>+  *aIsProbablyForLayout = PR_FALSE;
>+
>+  return NS_OK;
>+}

Are we going to make this smarter? (Otherwise perhaps inline it?)

>+
>+////////////////////////////////////////////////////////////////////////////////
>+// Protected
>+
>+PRBool
>+nsARIAGridAccessible::IsValidRow(PRInt32 aRow)
>+{
>+  if (aRow < 0)
>+    return PR_FALSE;
>+  
>+  PRInt32 rowCount = 0;
>+  GetColumns(&rowCount);

GetRows ;)

I'll review nsARIAGridCellAccessible next...

Comment 6

[David Bolter [:davidb] (NeedInfo me for attention)]

Comment on attachment 377363 [details] [diff] [review]
patch

>+nsresult
>+nsARIAGridCellAccessible::GetAttributesInternal(nsIPersistentProperties *aAttributes)
>+{

(See Neil's comment)

>+#endif
>\ No newline at end of file

I dunno if this is a problem?

For the mochitests they look good but we should probably add an aria grid where # rows != # cols ;)

I'd like to take a look at the new patch, so I'll r- for now.

Comment 7

[David Bolter [:davidb] (NeedInfo me for attention)]

(In reply to comment #5)
> (From update of attachment 377363 [details] [diff] [review])
> for that. Return the first one we find and bail -- next patch ;).

Alex, here I mean't to say "next phase" not next patch :)

Comment 8

[alexander :surkov (:asurkov)]

(In reply to comment #5)
> 
> >+   * XXX: not very well name property (header vs head).
> >+   */
> >   readonly attribute nsIAccessibleTable  rowHeader;
> 
> Not sure I understand the naming issue here. Is "columnHeader" bad too?

I changed my mind. It's not bad it's different. See below.

> 
> I think the assumption is there is one row header per table. We should just
> believe the author if they have a role="rowheader" and return the accessible
> for that. Return the first one we find and bail -- next patch ;).

IA2's rowheader and ARIA rowheader are different things, that was a reason I marked rowHeader property name as bad initially. IA2's rowheader is an accessible table containing ARIA rowheader elements. In the meantime we discuss with Pete on IA2 maillist what should we return here.

> GetRows right?  (Should probably add more tests)

Thank you for catching.

> >+  // XXX: should we rely on aria-selected or DOM selection?
> 
> To answer all these comments (for next patch) always let aria-* override
> native.

For getting I would say yes. What should we do when AT ask us to select?

> 
> >+NS_IMETHODIMP
> >+nsARIAGridAccessible::IsProbablyForLayout(PRBool *aIsProbablyForLayout)

> Are we going to make this smarter? (Otherwise perhaps inline it?)

Never. How to inline it?

Comment 9

[alexander :surkov (:asurkov)]

Attachment: patch2

Comment 10

[David Bolter [:davidb] (NeedInfo me for attention)]

Comment on attachment 377628 [details] [diff] [review]
patch2

This feels good.

r=me

(In reply to comment #8)
> (In reply to comment #5)

> > >+  // XXX: should we rely on aria-selected or DOM selection?
> > 
> > To answer all these comments (for next patch) always let aria-* override
> > native.
> 
> For getting I would say yes. What should we do when AT ask us to select?

I think aria wins here too.

> Never. How to inline it?

Actually the compiler should inline if it can so don't worry about it. Besides, inlining can cause bloat. (inlining by defining the function in the header file)

Nice work.

Comment 11

[alexander :surkov (:asurkov)]

(In reply to comment #10)
> (From update of attachment 377628 [details] [diff] [review])
> This feels good.
> 
> r=me
> 
> (In reply to comment #8)
> > (In reply to comment #5)
> 
> > > >+  // XXX: should we rely on aria-selected or DOM selection?
> > > 
> > > To answer all these comments (for next patch) always let aria-* override
> > > native.
> > 
> > For getting I would say yes. What should we do when AT ask us to select?
> 
> I think aria wins here too.

David, I think the question is not about winer actually. I realize we should set up/remove aria-selected but should we control the selection additionally or should we leave this for page author?

> 
> > Never. How to inline it?
> 
> Actually the compiler should inline if it can so don't worry about it. Besides,
> inlining can cause bloat. (inlining by defining the function in the header
> file)

Yes but we use macros generated by midl, therefore I'm not sure how to set inline without dropping macros usage.

Comment 12

[David Bolter [:davidb] (NeedInfo me for attention)]

(In reply to comment #11)
> (In reply to comment #10)
> > (From update of attachment 377628 [details] [diff] [review] [details])
> > This feels good.
> > 
> > r=me
> > 
> > (In reply to comment #8)
> > > (In reply to comment #5)
> > 
> > > > >+  // XXX: should we rely on aria-selected or DOM selection?
> > > > 
> > > > To answer all these comments (for next patch) always let aria-* override
> > > > native.
> > > 
> > > For getting I would say yes. What should we do when AT ask us to select?
> > 
> > I think aria wins here too.
> 
> David, I think the question is not about winer actually. I realize we should
> set up/remove aria-selected but should we control the selection additionally or
> should we leave this for page author?

I need to think about this. Do you have ideas here?

> 
> > 
> > > Never. How to inline it?
> > 
> > Actually the compiler should inline if it can so don't worry about it. Besides,
> > inlining can cause bloat. (inlining by defining the function in the header
> > file)
> 
> Yes but we use macros generated by midl, therefore I'm not sure how to set
> inline without dropping macros usage.

OK

Comment 13

[Marco Zehe (:MarcoZ)]

Comment on attachment 377628 [details] [diff] [review]
patch2

Cool stuff! R=me

Regarding the selection and aria-labelledby/aria-describedby to-do items, we should definitely reach out to the community to see what works best for them. I would think aria-selected would be the safest, then it's in control of the ARIA author. But that's just my 2 Cents.

Comment 14

[alexander :surkov (:asurkov)]

(In reply to comment #12)

> > David, I think the question is not about winer actually. I realize we should
> > set up/remove aria-selected but should we control the selection additionally or
> > should we leave this for page author?
> 
> I need to think about this. Do you have ideas here?

(In reply to comment #13)
> Regarding the selection and aria-labelledby/aria-describedby to-do items, we
> should definitely reach out to the community to see what works best for them. I
> would think aria-selected would be the safest, then it's in control of the ARIA
> author. But that's just my 2 Cents.

I would tend to agree we should use aria-selected only because widget author can care about selection himself and theoretically he can use CSS to style aria-selected nodes and not care about DOM selection at all. Though I'm not sure how much this approach is correct from point of view of widget implementation.

Comment 15

[alexander :surkov (:asurkov)]

checked in on 1.9.2 - http://hg.mozilla.org/mozilla-central/rev/44170cc1c68a