Last Comment Bug 777117 - provide TableCellAccessible interface
: provide TableCellAccessible interface
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla17
Assigned To: Trevor Saunders (:tbsaunde)
:
Mentors:
Depends on:
Blocks: dexpcoma11y
  Show dependency treegraph
 
Reported: 2012-07-24 15:37 PDT by Trevor Saunders (:tbsaunde)
Modified: 2012-07-30 21:25 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (16.90 KB, patch)
2012-07-24 15:39 PDT, Trevor Saunders (:tbsaunde)
dbolter: review+
Details | Diff | Review

Description Trevor Saunders (:tbsaunde) 2012-07-24 15:37:55 PDT
same as bug 648265 for table cells
Comment 1 Trevor Saunders (:tbsaunde) 2012-07-24 15:39:38 PDT
Created attachment 645541 [details] [diff] [review]
patch

seems reasonable to use exactly the same approach as tables.
Comment 2 Trevor Saunders (:tbsaunde) 2012-07-24 17:31:01 PDT
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) 2012-07-29 20:51:26 PDT
landed https://hg.mozilla.org/integration/mozilla-inbound/rev/4c8cfae37b56
Comment 4 Ed Morley [:emorley] 2012-07-30 02:43:57 PDT
https://hg.mozilla.org/mozilla-central/rev/4c8cfae37b56
Comment 5 Justin Dolske [:Dolske] 2012-07-30 13:26:53 PDT
Long checkin comment is long. :-(
Comment 6 alexander :surkov 2012-07-30 21:25:38 PDT
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

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