777117 - provide TableCellAccessible interface

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Trevor Saunders (:tbsaunde)]

same as bug 648265 for table cells

Comment 1

[Trevor Saunders (:tbsaunde)]

Attachment: patch

seems reasonable to use exactly the same approach as tables.

Comment 2

[Trevor Saunders (:tbsaunde)]

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.

Comment 3

[Trevor Saunders (:tbsaunde)]

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

Comment 4

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/4c8cfae37b56

Comment 5

[Justin Dolske [:Dolske]]

Long checkin comment is long. :-(

Comment 6

[alexander :surkov (:asurkov)]

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