decomtaminate GetSelectedCells() on accessible tables

RESOLVED FIXED in mozilla16

Status

()

Core
Disability Access APIs
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: capella, Assigned: Xi Yang)

Tracking

unspecified
mozilla16
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][mentor=trev.saunders@gmail.com][lang=c++])

Attachments

(1 attachment, 1 obsolete attachment)

18.79 KB, patch
tbsaunde
: review+
Details | Diff | Splinter Review
(Reporter)

Description

5 years ago
Last method for xpcaccessibletable to be decomtaminated.

See bug 765512, bug 747227, etc.
(Reporter)

Updated

5 years ago
Assignee: markcapella → nobody
Status: ASSIGNED → NEW
(Assignee)

Comment 1

5 years ago
Hi, could someone assign the bug to me?
(Reporter)

Updated

5 years ago
Assignee: nobody → vinceyang15
Status: NEW → ASSIGNED
(Assignee)

Comment 2

5 years ago
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?
(Reporter)

Comment 3

5 years ago
Xi, post your patch as it is so it can be looked at, hard to tell what you did wrong otherwise.
(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
(Assignee)

Comment 5

5 years ago
Created attachment 637088 [details] [diff] [review]
Patch(v1)
Attachment #637088 - Flags: review?(trev.saunders)
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.
Attachment #637088 - Flags: review?(trev.saunders)
(Assignee)

Comment 7

5 years ago
Created attachment 637478 [details] [diff] [review]
Patch(v2)
Attachment #637088 - Attachment is obsolete: true
Attachment #637478 - Flags: review?(trev.saunders)
Attachment #637478 - Flags: review?(trev.saunders) → review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
(Reporter)

Comment 8

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

caused WIN7 build to fail ...
(Reporter)

Comment 9

5 years ago
Pushing to TRY:
https://tbpl.mozilla.org/?tree=Try&rev=3a22977271ae
Keywords: checkin-needed
(Reporter)

Comment 10

5 years ago
On to inbound:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=bdafb1929d8d
(Assignee)

Comment 11

5 years ago
(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?
(Reporter)

Comment 12

5 years ago
I just removed the extra space ... mentioned it here as it was an odd compiler thing ... fyi thing ... s/b complete / production soon ...
https://hg.mozilla.org/mozilla-central/rev/bdafb1929d8d
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
You need to log in before you can comment on or make changes to this bug.