767705 - decomtaminate GetSelectedCells() on accessible tables

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Mark Capella [:capella]]

Last method for xpcaccessibletable to be decomtaminated.

See bug 765512, bug 747227, etc.

Comment 1

[Xi Yang]

Hi, could someone assign the bug to me?

Comment 2

[Xi Yang]

I just got this problem. I've implemented the xpcAccessibleTable::GetSelectedCells(nsIArray**) method, and I tried to compile my code, but the compiler just saying error: 'nsIArray' has not been declared, How can I fix it?

Comment 3

[Mark Capella [:capella]]

Xi, post your patch as it is so it can be looked at, hard to tell what you did wrong otherwise.

Comment 4

[Trevor Saunders (:tbsaunde)]

(In reply to Xi Yang from comment #2)
> I just got this problem. I've implemented the
> xpcAccessibleTable::GetSelectedCells(nsIArray**) method, and I tried to
> compile my code, but the compiler just saying error: 'nsIArray' has not been
> declared, How can I fix it?

you probably need to include nsArrayUtils.h

Comment 5

[Xi Yang]

Attachment: Patch(v1)

Comment 6

[Trevor Saunders (:tbsaunde)]

Comment on attachment 637088 [details] [diff] [review]
Patch(v1)

>+xpcAccessibleTable::GetSelectedCells(nsIArray** aSelectedCell)
>+{

nit, aSelectedCells here and below.

>+  nsAutoTArray<Accessible*, XPC_TABLE_DEFAULT_SIZE> cellsArray;
>+  mTable->SelectedCells(&cellsArray);
>+
>+  PRUint32 aCellsArraySize = cellsArray.Length();
>+  *aSelectedCell = static_cast<nsIArray*>
>+    (moz_xmalloc(aCellsArraySize * sizeof(nsIArray*)));
>+  memcpy(*aSelectedCell, cellsArray.Elements(),
>+         aCellsArraySize * sizeof(nsIArray*));

that isn't how you create a nsIArray... look at some of the code you remove for an example of how to do it correctly.

>diff --git a/accessible/src/xpcom/xpcAccessibleTable.h b/accessible/src/xpcom/xpcAccessibleTable.h
>--- a/accessible/src/xpcom/xpcAccessibleTable.h
>+++ b/accessible/src/xpcom/xpcAccessibleTable.h
>@@ -4,16 +4,18 @@
>  * 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_XPACCESSIBLETABLE_H_
> #define MOZILLA_A11Y_XPCOM_XPACCESSIBLETABLE_H_
> 
> #include "nsAString.h"
> #include "nscore.h"
>+#include "nsArrayUtils.h"

a forward decl of class nsIArray should be enough here. then you can include it in xpcAccessibleTable.cpp

>   PRUint32 selectedItemsCount = 0;
>+  nsresult rv;
>   rv = selectedItems->GetLength(&selectedItemsCount);

nit, just do nsresult rv = ...

>+  if (!mDoc) 
>+    return;

you don't need this check.

Comment 7

[Xi Yang]

Attachment: Patch(v2)

Comment 8

[Mark Capella [:capella]]

nit, a trailing space here:
-    { return xpcAccessibleTable::GetSelectedCells(aSelectedCells); } \ 
+    { return xpcAccessibleTable::GetSelectedCells(aSelectedCells); } \

caused WIN7 build to fail ...

Comment 9

[Mark Capella [:capella]]

Pushing to TRY:
https://tbpl.mozilla.org/?tree=Try&rev=3a22977271ae

Comment 10

[Mark Capella [:capella]]

On to inbound:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=bdafb1929d8d

Comment 11

[Xi Yang]

(In reply to Mark Capella [:capella] from comment #8)
> nit, a trailing space here:
> -    { return xpcAccessibleTable::GetSelectedCells(aSelectedCells); } \ 
> +    { return xpcAccessibleTable::GetSelectedCells(aSelectedCells); } \
> 
> caused WIN7 build to fail ...

How to fix it?

Comment 12

[Mark Capella [:capella]]

I just removed the extra space ... mentioned it here as it was an odd compiler thing ... fyi thing ... s/b complete / production soon ...

Comment 13

[Ryan VanderMeulen [:RyanVM]]

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