380508 - implement IAccessibleHyperlink

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 1

[[e-mail]]

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

>+class CAccessibleAction: public nsISupports,
I know you do this so that you can use do_QueryInterface, but you should know that this bloats your object's memory footprint, although since you already have at least 12 vtable entries, an extra one or two wouldn't seem to matter.

>+  void *instancePtr = NULL;
>+  nsresult rv =  winAccessNode->QueryNativeInterface(IID_IAccessible2,
>+                                                     &instancePtr);
>+  if (NS_FAILED(rv))
>+    return E_FAIL;
>+
>+  IUnknown *unknownPtr = NS_STATIC_CAST(IUnknown*, instancePtr);
>+   aAnchor->ppunkVal = &unknownPtr;
You queried for IID_Iaccessible2, not IUnknown? Also your indent is wrong.

Comment 2

[alexander :surkov (:asurkov)]

(In reply to comment #1)
> (From update of attachment 264603 [details] [diff] [review])
> >+class CAccessibleAction: public nsISupports,
> I know you do this so that you can use do_QueryInterface, but you should know
> that this bloats your object's memory footprint, although since you already
> have at least 12 vtable entries, an extra one or two wouldn't seem to matter.

How can I avoid this?

> >+  void *instancePtr = NULL;
> >+  nsresult rv =  winAccessNode->QueryNativeInterface(IID_IAccessible2,
> >+                                                     &instancePtr);
> >+  if (NS_FAILED(rv))
> >+    return E_FAIL;
> >+
> >+  IUnknown *unknownPtr = NS_STATIC_CAST(IUnknown*, instancePtr);
> >+   aAnchor->ppunkVal = &unknownPtr;
> You queried for IID_Iaccessible2, not IUnknown? Also your indent is wrong.
> 

IAccessible2 is inherited from IUnknown. I query IAccessible2 to be sure I return accessible object.

Comment 3

[[e-mail]]

(In reply to comment #2)
>(In reply to comment #1)
>>this bloats your object's memory footprint, although since you already
>>have at least 12 vtable entries, an extra one or two wouldn't seem to matter.
>How can I avoid this?
With difficulty. You would have to declare NS_IMETHOD QueryInterface(REFNSIID aIID, void** aInstancePtr) = 0; in the class, and call QueryInterface(NS_GET_IID(nsIAccessible), getter_AddRefs(acc));

>>>+  void *instancePtr = NULL;
>>>+  nsresult rv =  winAccessNode->QueryNativeInterface(IID_IAccessible2,
>>>+                                                     &instancePtr);
>>>+  if (NS_FAILED(rv))
>>>+    return E_FAIL;
>>>+
>>>+  IUnknown *unknownPtr = NS_STATIC_CAST(IUnknown*, instancePtr);
>>>+   aAnchor->ppunkVal = &unknownPtr;
>>You queried for IID_Iaccessible2, not IUnknown? Also your indent is wrong.
>IAccessible2 is inherited from IUnknown. I query IAccessible2 to be sure I
>return accessible object.
But the IUnknown you get from casting IAccessible2 might not be the same IUnknown that you get when you QueryNativeInterface for it directly.

Comment 4

[alexander :surkov (:asurkov)]

(In reply to comment #3)
> (In reply to comment #2)
> >(In reply to comment #1)
> >>this bloats your object's memory footprint, although since you already
> >>have at least 12 vtable entries, an extra one or two wouldn't seem to matter.
> >How can I avoid this?
> With difficulty. You would have to declare NS_IMETHOD QueryInterface(REFNSIID
> aIID, void** aInstancePtr) = 0; in the class, and call
> QueryInterface(NS_GET_IID(nsIAccessible), getter_AddRefs(acc));

I filed another bug for this because it's not issue of CAccessibleAction only.

> >>>+  void *instancePtr = NULL;
> >>>+  nsresult rv =  winAccessNode->QueryNativeInterface(IID_IAccessible2,
> >>>+                                                     &instancePtr);
> >>>+  if (NS_FAILED(rv))
> >>>+    return E_FAIL;
> >>>+
> >>>+  IUnknown *unknownPtr = NS_STATIC_CAST(IUnknown*, instancePtr);
> >>>+   aAnchor->ppunkVal = &unknownPtr;
> >>You queried for IID_Iaccessible2, not IUnknown? Also your indent is wrong.
> >IAccessible2 is inherited from IUnknown. I query IAccessible2 to be sure I
> >return accessible object.
> But the IUnknown you get from casting IAccessible2 might not be the same
> IUnknown that you get when you QueryNativeInterface for it directly.
> 

Ok, I'll fix it. But why do this happen?

Comment 5

[alexander :surkov (:asurkov)]

(In reply to comment #4)

> I filed another bug for this because it's not issue of CAccessibleAction only.

The bug 380836.

Comment 6

[alexander :surkov (:asurkov)]

Attachment: patch2

for checking in

Comment 7

[alexander :surkov (:asurkov)]

checked in

Comment 8

[[e-mail]]

(In reply to comment #4)
>>But the IUnknown you get from casting IAccessible2 might not be the same
>>IUnknown that you get when you QueryNativeInterface for it directly.
>Ok, I'll fix it. But why do this happen?
Oh, it seems there's a minor error in nsAccessibleWrap::QueryInterface
Since IAccessible2 is a subclass of IAccessible it should look like this:
>  if (IID_IUnknown == iid || IID_IDispatch == iid || IID_IAccessible == iid || IID_IAccessible2 == iid)
>    *ppv = NS_STATIC_CAST(IAccessible2*, this);
This means that your original code will actually work, but it's still obscure.