Last Comment Bug 767705 - decomtaminate GetSelectedCells() on accessible tables
: decomtaminate GetSelectedCells() on accessible tables
Status: RESOLVED FIXED
[good first bug][mentor=trev.saunders...
:
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla16
Assigned To: Xi Yang
:
:
Mentors:
Depends on:
Blocks: dexpcoma11y
  Show dependency treegraph
 
Reported: 2012-06-23 06:37 PDT by Mark Capella [:capella]
Modified: 2012-06-30 12:46 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch(v1) (18.16 KB, patch)
2012-06-27 05:55 PDT, Xi Yang
no flags Details | Diff | Splinter Review
Patch(v2) (18.79 KB, patch)
2012-06-28 05:58 PDT, Xi Yang
tbsaunde+mozbugs: review+
Details | Diff | Splinter Review

Description Mark Capella [:capella] 2012-06-23 06:37:39 PDT
Last method for xpcaccessibletable to be decomtaminated.

See bug 765512, bug 747227, etc.
Comment 1 Xi Yang 2012-06-25 02:37:23 PDT
Hi, could someone assign the bug to me?
Comment 2 Xi Yang 2012-06-26 07:52:11 PDT
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] 2012-06-26 08:00:30 PDT
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) 2012-06-26 08:03:31 PDT
(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 2012-06-27 05:55:02 PDT
Created attachment 637088 [details] [diff] [review]
Patch(v1)
Comment 6 Trevor Saunders (:tbsaunde) 2012-06-27 11:35:04 PDT
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 2012-06-28 05:58:37 PDT
Created attachment 637478 [details] [diff] [review]
Patch(v2)
Comment 8 Mark Capella [:capella] 2012-06-28 15:04:27 PDT
nit, a trailing space here:
-    { return xpcAccessibleTable::GetSelectedCells(aSelectedCells); } \ 
+    { return xpcAccessibleTable::GetSelectedCells(aSelectedCells); } \

caused WIN7 build to fail ...
Comment 9 Mark Capella [:capella] 2012-06-28 15:46:08 PDT
Pushing to TRY:
https://tbpl.mozilla.org/?tree=Try&rev=3a22977271ae
Comment 10 Mark Capella [:capella] 2012-06-29 01:17:12 PDT
On to inbound:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=bdafb1929d8d
Comment 11 Xi Yang 2012-06-30 03:14:24 PDT
(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] 2012-06-30 09:51:06 PDT
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] 2012-06-30 12:46:26 PDT
https://hg.mozilla.org/mozilla-central/rev/bdafb1929d8d

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