829382 - remove nsIAccessibleTableCell usage from ia2AccessibleTableCell

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Comment 1

[Trevor Saunders (:tbsaunde)]

Attachment: remove nsIWinAccessNode and stuff from ia2AccessibleTableCell

Comment 2

[alexander :surkov (:asurkov)]

Comment on attachment 700783 [details] [diff] [review]
remove nsIWinAccessNode and stuff from ia2AccessibleTableCell

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

::: accessible/src/windows/ia2/ia2AccessibleTableCell.cpp
@@ +46,5 @@
>  ia2AccessibleTableCell::get_table(IUnknown** aTable)
>  {
>    A11Y_TRYBLOCK_BEGIN
>  
> +    *aTable = NULL;

nit: wrong indent

@@ +84,5 @@
>                                                long* aNColumnHeaderCells)
>  {
>    A11Y_TRYBLOCK_BEGIN
>  
> +    *aCellAccessibles = NULL;

nit: wrong indetn

@@ +95,5 @@
> +
> +  *aNColumnHeaderCells = cells.Length();
> +  *aCellAccessibles =
> +    static_cast<IUnknown**>(::CoTaskMemAlloc(sizeof(IUnknown*) *
> +                                             cells.Length()));

null check?

@@ +99,5 @@
> +                                             cells.Length()));
> +
> +  for (uint32_t i = 0; i < cells.Length(); i++) {
> +    AccessibleWrap* cell = static_cast<AccessibleWrap*>(cells[i]);
> +    (*aCellAccessibles[i] = static_cast<IAccessible*>(cell))->AddRef();

I prefer having AddRef() as separate statement (here and below)

@@ +143,5 @@
>                                             long* aNRowHeaderCells)
>  {
>    A11Y_TRYBLOCK_BEGIN
>  
> +    *aCellAccessibles = NULL;

nit: wrong indent

@@ +154,5 @@
> +
> +  *aNRowHeaderCells = cells.Length();
> +  *aCellAccessibles =
> +    static_cast<IUnknown**>(::CoTaskMemAlloc(sizeof(IUnknown*) *
> +                                             cells.Length()));

null check, right?

@@ +189,5 @@
>                                               boolean* aIsSelected)
>  {
>    A11Y_TRYBLOCK_BEGIN
>  
> +    *aRowIdx = *aColIdx = *aRowExtents = *aColExtents = 0;

nit: wrong indent

::: accessible/src/windows/ia2/ia2AccessibleTableCell.h
@@ +15,5 @@
> +namespace mozilla {
> +namespace a11y {
> +class TableCellAccessible;
> +}
> +}

please put it under a11y namespace as well

@@ +66,5 @@
> +protected:
> +  ia2AccessibleTableCell(mozilla::a11y::TableCellAccessible* aTableCell) :
> +    mTableCell(aTableCell) {}
> +
> +  mozilla::a11y::TableCellAccessible* mTableCell;

note, in that case you don't need these prefixes

Comment 3

[Trevor Saunders (:tbsaunde)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ead76326763b

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ead76326763b