Closed
Bug 835121
Opened 11 years ago
Closed 11 years ago
ARIA grid should be editable by default
Categories
(Core :: Disability Access APIs, defect)
Core
Disability Access APIs
Tracking
()
RESOLVED
FIXED
mozilla21
People
(Reporter: surkov, Assigned: surkov)
References
(Blocks 1 open bug)
Details
(Keywords: access)
Attachments
(1 file, 1 obsolete file)
9.61 KB,
patch
|
tbsaunde
:
review+
|
Details | Diff | Splinter Review |
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•11 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•11 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•11 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•11 years ago
|
||
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #709366 -
Flags: review?(trev.saunders)
Assignee | ||
Comment 5•11 years ago
|
||
related ARIA failure tracking URL: https://wiki.mozilla.org/Accessibility/ARIA1.0TestSuiteFailures#182
Comment 6•11 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•11 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 8•11 years ago
|
||
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•11 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.
Comment 10•11 years ago
|
||
(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•11 years ago
|
||
I see, misread you
Comment 12•11 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•11 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•11 years ago
|
||
ping?
Comment 15•11 years ago
|
||
(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•11 years ago
|
||
Attachment #709366 -
Attachment is obsolete: true
Attachment #709366 -
Flags: review?(trev.saunders)
Attachment #712791 -
Flags: review?(trev.saunders)
Updated•11 years ago
|
Attachment #712791 -
Flags: review?(trev.saunders) → review+
Assignee | ||
Comment 17•11 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/55a453689fb1
Flags: in-testsuite+
Comment 18•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/55a453689fb1
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in
before you can comment on or make changes to this bug.
Description
•