Closed Bug 835121 Opened 11 years ago Closed 11 years ago

ARIA grid should be editable by default

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: surkov, Assigned: surkov)

References

(Blocks 1 open bug)

Details

(Keywords: access)

Attachments

(1 file, 1 obsolete file)

Spec says (http://www.w3.org/TR/wai-aria/roles#grid):

"A grid is considered editable unless otherwise specified. To make a grid read-only, set the aria-readonly attribute of the grid to true. The value of the grid element's aria-readonly attribute is implicitly propagated to all of its owned gridcell elements, and will be exposed through the accessibility API. An author may override an individual gridcell element's propagated aria-readonly value by setting the aria-readonly attribute on the gridcell."

I wonder how useful the editable state on grid elements is, in other words I wonder whether editable state on cells is enough.

Jamie, what will you say?
I don't really have a preference here. For the most part, it doesn't really matter. My gut feeling is that the grid probably should have the editable state, but I can't really justify that. I guess the grid having the editable state suggests that the grid itself is editable (i.e. you can add cells to it, etc.; e.g. a spreadsheet), whereas the editable state on a cell only suggests that particular cell is editable (i.e. its content).
Oh, that makes sense. 

Do you think should editable state be propagated to each cell from a grid until the cell overrides it?
(In reply to alexander :surkov from comment #2)
> Do you think should editable state be propagated to each cell from a grid
> until the cell overrides it?
Yes.
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #709366 - Flags: review?(trev.saunders)
Hi alex, does this mean that when role=grid is used as it sometimes is for creating read onlt data tables out of arbitrary elements, the cells will be editbale by default?
(In reply to steve faulkner from comment #6)
> Hi alex, does this mean that when role=grid is used as it sometimes is for
> creating read onlt data tables out of arbitrary elements, the cells will be
> editbale by default?

yes, is that not expected?
Comment on attachment 709366 [details] [diff] [review]
patch

>+  if (mRoleMapEntry->Is(nsGkAtoms::gridcell) &&
>+      !(*aState & (states::READONLY | states::EDITABLE))) {
>+    TableCellAccessible* cell = const_cast<Accessible*>(this)->AsTableCell();

why not add const TablecellAccessible* AsTableCell() const; ?
(In reply to Trevor Saunders (:tbsaunde) from comment #8)
> Comment on attachment 709366 [details] [diff] [review]
> patch
> 
> >+  if (mRoleMapEntry->Is(nsGkAtoms::gridcell) &&
> >+      !(*aState & (states::READONLY | states::EDITABLE))) {
> >+    TableCellAccessible* cell = const_cast<Accessible*>(this)->AsTableCell();
> 
> why not add const TablecellAccessible* AsTableCell() const; ?

would that work? The problem here is this is const because we are in ApplyARIAState() but AsTableCell() is not const.
(In reply to alexander :surkov from comment #9)
> (In reply to Trevor Saunders (:tbsaunde) from comment #8)
> > Comment on attachment 709366 [details] [diff] [review]
> > patch
> > 
> > >+  if (mRoleMapEntry->Is(nsGkAtoms::gridcell) &&
> > >+      !(*aState & (states::READONLY | states::EDITABLE))) {
> > >+    TableCellAccessible* cell = const_cast<Accessible*>(this)->AsTableCell();
> > 
> > why not add const TablecellAccessible* AsTableCell() const; ?
> 
> would that work? The problem here is this is const because we are in
> ApplyARIAState() but AsTableCell() is not const.

I'm suggesting adding a AsTableCell() that is const which you should be able to do so long as it returns a const TableCellAccessible*
I see, misread you
(In reply to alexander :surkov from comment #7)
> (In reply to steve faulkner from comment #6)
> > Hi alex, does this mean that when role=grid is used as it sometimes is for
> > creating read onlt data tables out of arbitrary elements, the cells will be
> > editbale by default?
> 
> yes, is that not expected?

ignore me :-) , I pinged Rich offline to clarify.
(In reply to Trevor Saunders (:tbsaunde) from comment #8)

> why not add const TablecellAccessible* AsTableCell() const; ?

that means two independent implementations of AsTableCell() or are you fine to have a second version as inline thing of the first one using const_cast again?
ping?
(In reply to alexander :surkov from comment #13)
> (In reply to Trevor Saunders (:tbsaunde) from comment #8)
> 
> > why not add const TablecellAccessible* AsTableCell() const; ?
> 
> that means two independent implementations of AsTableCell() or are you fine
> to have a second version as inline thing of the first one using const_cast
> again?

personally I'd probably just provide seperate impl afterall its a trivial method, but either's fine with me I think.
Attached patch patch2Splinter Review
Attachment #709366 - Attachment is obsolete: true
Attachment #709366 - Flags: review?(trev.saunders)
Attachment #712791 - Flags: review?(trev.saunders)
Attachment #712791 - Flags: review?(trev.saunders) → review+
https://hg.mozilla.org/mozilla-central/rev/55a453689fb1
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Depends on: 857936
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: