provide TableCellAccessible interface

RESOLVED FIXED in mozilla17

Status

()

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

People

(Reporter: tbsaunde, Assigned: tbsaunde)

Tracking

unspecified
mozilla17
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
same as bug 648265 for table cells
(Assignee)

Comment 1

5 years ago
Created attachment 645541 [details] [diff] [review]
patch

seems reasonable to use exactly the same approach as tables.
Attachment #645541 - Flags: review?(dbolter)
(Assignee)

Comment 2

5 years ago
for those curious nullptr was because it looks like bug 626472 is about to land.

looking at PR*nt32 or {,u}int32_t as a rough approximation I found

tbsaunde@exception:~/src/mozilla-central$ ack-grep  --ignore-dir=js/src --ignore-dir=obj-x86_64-unknown-linux-gnu/ uint32_t | wc   7377   41354  685657
tbsaunde@exception:~/src/mozilla-central$ ack-grep  --ignore-dir=js/src --ignore-dir=obj-x86_64-unknown-linux-gnu/ PRUint32 | wc  29194  155545 2683064
tbsaunde@exception:~/src/mozilla-central$ ack-grep  --ignore-dir=js/src --ignore-dir=obj-x86_64-unknown-linux-gnu/ int32_t | wc   11504   62554 1092793
tbsaunde@exception:~/src/mozilla-central$ ack-grep  --ignore-dir=js/src --ignore-dir=obj-x86_64-unknown-linux-gnu/ PRInt32 | wc
  24131  121541 2227123

so, while there is certainly more nspr types the standard ones seem to be popular enough I don't think more of them will be too out of place.
Attachment #645541 - Flags: review?(dbolter) → review+
(Assignee)

Comment 3

5 years ago
landed https://hg.mozilla.org/integration/mozilla-inbound/rev/4c8cfae37b56

Comment 4

5 years ago
https://hg.mozilla.org/mozilla-central/rev/4c8cfae37b56
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Long checkin comment is long. :-(

Comment 6

5 years ago
Comment on attachment 645541 [details] [diff] [review]
patch

Review of attachment 645541 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/src/generic/TableCellAccessible.h
@@ +14,5 @@
> +
> +namespace mozilla {
> +namespace a11y {
> +
> +  class TableAccessible;

extra space

@@ +17,5 @@
> +
> +  class TableAccessible;
> +
> +/**
> + * abstract interface implemented by table cell accessibles.

'a' in uppercase

@@ +26,5 @@
> +
> +  /**
> +   * Return the table this cell is in.
> +   */
> +  virtual TableAccessible* Table() { return nullptr; }

these getters should be const, at least we should try to keep them this way

@@ +29,5 @@
> +   */
> +  virtual TableAccessible* Table() { return nullptr; }
> +
> +  /**
> +   * Return the Column of the table this cell is in.

Column -> column

@@ +65,5 @@
> +  virtual bool Selected() { return false; }
> +};
> +
> +}
> +}

there's no commented namespaces

::: accessible/src/html/HTMLTableAccessible.cpp
@@ +59,5 @@
>  ////////////////////////////////////////////////////////////////////////////////
>  // HTMLTableCellAccessible: Accessible implementation
>  
> +  void
> +  HTMLTableCellAccessible::Shutdown()

nit: extra spaces

::: accessible/src/xpcom/xpcAccessibleTableCell.h
@@ +4,5 @@
> + * License, v. 2.0. If a copy of the MPL was not distributed with this file,
> + * You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +#ifndef MOZILLA_A11Y_XPCOM_XPACESSIBLETABLECELL_H_
> +#define MOZILLA_A11Y_XPCOM_XPACESSIBLETABLECELL_H_

nit: capslock style is not consistent to what you did above

@@ +13,5 @@
> +class TableCellAccessible;
> +}
> +}
> +
> +class xpcAccessibleTableCell

it should be ok to keep it inside mozilla::a11y namespace?

::: accessible/src/xul/XULTreeGridAccessible.cpp
@@ +498,5 @@
>  ////////////////////////////////////////////////////////////////////////////////
>  // XULTreeGridCellAccessible: nsIAccessible implementation
>  
> +  void
> +  XULTreeGridCellAccessible::Shutdown()

extra spaces
You need to log in before you can comment on or make changes to this bug.