ARIA grid should be editable by default

RESOLVED FIXED in mozilla21

Status

()

Core
Disability Access APIs
RESOLVED FIXED
5 years ago
4 months ago

People

(Reporter: surkov, Assigned: surkov)

Tracking

(Blocks: 1 bug, {access})

unspecified
mozilla21
access
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

9.61 KB, patch
tbsaunde
: review+
Details | Diff | Splinter Review
(Assignee)

Description

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

Comment 1

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

Comment 2

5 years ago
Oh, that makes sense. 

Do you think should editable state be propagated to each cell from a grid until the cell overrides it?

Comment 3

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

Comment 4

5 years ago
Created attachment 709366 [details] [diff] [review]
patch
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #709366 - Flags: review?(trev.saunders)
(Assignee)

Comment 5

5 years ago
related ARIA failure tracking URL: https://wiki.mozilla.org/Accessibility/ARIA1.0TestSuiteFailures#182

Comment 6

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

Comment 7

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

Comment 9

5 years ago
(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*
(Assignee)

Comment 11

5 years ago
I see, misread you

Comment 12

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

Comment 13

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

Comment 14

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

Comment 16

5 years ago
Created attachment 712791 [details] [diff] [review]
patch2
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+
(Assignee)

Comment 17

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/55a453689fb1
Flags: in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/55a453689fb1
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21

Updated

4 years ago
Depends on: 857936
See Also: → bug 1359482
You need to log in before you can comment on or make changes to this bug.