461920 - remove nsPIAccessNode

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Comment 1

[alexander :surkov (:asurkov)]

Attachment: patch

is there a nice way to query from nsCOMPtr to nsRefPtr?

Comment 2

[[e-mail]]

While looking for one I found a leak in nsAccessible::GetAttributesInternal

nsCOMPtr<nsISupports> container = doc->GetContainer();
nsIDocShellTreeItem *docShellTreeItem = nsnull;
if (container)
  CallQueryInterface(container, &docShellTreeItem);

docShellTreeItem is never released. Correct coding here is:

nsCOMPtr<nsIDocShellTreeItem>
    docShellTreeItem(do_QueryInterface(doc->GetContainer()));

The common pattern seems to be to use a helper function:

already_AddRefed<nsAccessNode> GetAccessNode(nsIAccessSomething* aAccessible)
{
  nsAccessNode* accessNode = nsnull;
  if (aAccessible)
    CallQueryInterface(aAccessible, &accessNode);
  return accessNode;
}

Then you can use

nsRefPtr<nsAccessNode> accessNode = GetAccessNode(aAccessible);

Comment 3

[Aaron Leventhal]

Comment on attachment 345294 [details] [diff] [review]
patch

Waiting for new patch.

Comment 4

[alexander :surkov (:asurkov)]

Attachment: patch2

with neil's comments

Comment 5

[alexander :surkov (:asurkov)]

Comment on attachment 345425 [details] [diff] [review]
patch2

Marco, can you test this patch in real work?

Comment 6

[alexander :surkov (:asurkov)]

Attachment: patch3

Comment 7

[alexander :surkov (:asurkov)]

tryserver builds https://build.mozilla.org/tryserver-builds/2008-10-29_21:[e-mail]-1225341298/

Comment 8

[Marco Zehe (:MarcoZ)]

So far no negative impact in the three supporting screen readers on Windows. Both web pages and XUL dialogs work as they did before.

Comment 9

[Marco Zehe (:MarcoZ)]

Comment on attachment 345429 [details] [diff] [review]
patch3

All's fine on Linux as well. From a functionality standpoint, this is fine. Thanks!

Comment 10

[[e-mail]]

Comment on attachment 345429 [details] [diff] [review]
patch3

>-NS_IMPL_ISUPPORTS_INHERITED2(nsTextAccessible, nsAccessNode, nsIAccessible, nsPIAccessible)
>+NS_IMPL_ISUPPORTS_INHERITED3(nsTextAccessible, nsAccessNode,
>+                             nsAccessible, nsIAccessible, nsPIAccessible)
This looks like a fix left over from another patch ;-)

Comment 11

[alexander :surkov (:asurkov)]

(In reply to comment #10)
> (From update of attachment 345429 [details] [diff] [review])
> >-NS_IMPL_ISUPPORTS_INHERITED2(nsTextAccessible, nsAccessNode, nsIAccessible, nsPIAccessible)
> >+NS_IMPL_ISUPPORTS_INHERITED3(nsTextAccessible, nsAccessNode,
> >+                             nsAccessible, nsIAccessible, nsPIAccessible)
> This looks like a fix left over from another patch ;-)

No actually. You could argue it's another bug (because generally we don't deal here with nsAccessible) but I included it because this patch without this fix makes crash firefox.

Comment 12

[alexander :surkov (:asurkov)]

http://hg.mozilla.org/mozilla-central/rev/7684dee590b9