634829 - remove nsIAccessibleWin32Object interface

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

it's not needed, static nsAccesssibleWrap::GetHWNDFor can be used instead (change NativeAccessible to take nsAccessbile as an argument).

Comment 1

[Andrew Quartey [:drexler]]

Attachment: Patch v1

https://tbpl.mozilla.org/?tree=Try&rev=ff3e7cef0591

Comment 2

[Hubert Figuiere [:hub]]

Comment on attachment 582570 [details] [diff] [review]
Patch v1

Review of attachment 582570 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/src/msaa/nsAccessibleWrap.cpp
@@ +1721,5 @@
>     NS_WARNING("Not passing in an aXPAccessible");
>     return NULL;
>    }
> +  
> +  nsAccessible *accObject = dynamic_cast<nsAccessible*>(aXPAccessible);

coding style: '*' stuck to the type and space after. Applies in the following lines too.

Comment 3

[Andrew Quartey [:drexler]]

Attachment: Patch v2

Coding style changes per Hub's suggestion.

Comment 4

[Trevor Saunders (:tbsaunde)]

Comment on attachment 582591 [details] [diff] [review]
Patch v2


>+IDispatch* nsAccessibleWrap::NativeAccessible(nsIAccessible *aXPAccessible)

Per bug description  change to take an nsAccessible*

>+  nsAccessible* accObject = dynamic_cast<nsAccessible*>

(aXPAccessible);

isn't needed if you change the argument, but isn't correct since mozilla doesn't use rtti.

Comment 5

[Andrew Quartey [:drexler]]

Attachment: Patch v3

Changed NativeAccessible to take an nsAccessible* 

> 
>  but isn't correct since mozilla doesn't use rtti.
Interesting. That C++ portability guide could do with a little more visibility on the developer guide home page.

Comment 6

[Trevor Saunders (:tbsaunde)]

Comment on attachment 582744 [details] [diff] [review]
Patch v3

So it appears mHWND in nsHTMLWin32ObjectAccessible is now unused which means a lot more code there to get the HWND can go away.  However I'm not familiar enough with win32 and plugins to be sure that this change won't break something there.

Comment 7

[Marco Zehe (:MarcoZ)]

A try-server build would be helpful to verify this.

Comment 8

[Marco Zehe (:MarcoZ)]

Comment on attachment 582744 [details] [diff] [review]
Patch v3

1. This patch no longer applies against mozilla-central trunk.
2. After manually rebasing it and doing a local build, when I start that build, NVDA is no longer able to interact with any of the UI elements. parent-child-traversal from the primary application frame into the accessible tree is not possible, no virtual documents, nothing works.

Comment 9

[Andrew Quartey [:drexler]]

Interesting. I will update my working directory and send this patch to try.

Comment 10

[alexander :surkov (:asurkov)]

(In reply to alexander :surkov from comment #0)
> it's not needed, static nsAccesssibleWrap::GetHWNDFor can be used instead
> (change NativeAccessible to take nsAccessbile as an argument).

that doesn't work, sorry for confusion.

you need to override GetNativeInterface for nsHTMLWin32ObjectAccessible to return desired accessible. please let me know if you need details.

Comment 11

[Andrew Quartey [:drexler]]

Attachment: Patch v4

>that doesn't work, sorry for confusion.
No problem.

Comment 12

[alexander :surkov (:asurkov)]

Comment on attachment 595909 [details] [diff] [review]
Patch v4

Review of attachment 595909 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/src/msaa/nsHTMLWin32ObjectAccessible.cpp
@@ +121,3 @@
>  {
> +    *aOutAccessible = static_cast<IAccessible*>(this);
> +    NS_ADDREF_THIS();

hah, you should move here what you removed from nsAccessibleWrap::NativeAccessible

Comment 13

[Andrew Quartey [:drexler]]

Attachment: Patch v5

Comment 14

[alexander :surkov (:asurkov)]

Comment on attachment 595941 [details] [diff] [review]
Patch v5

Review of attachment 595941 [details] [diff] [review]:
-----------------------------------------------------------------

r=me

::: accessible/src/msaa/nsAccessibleWrap.cpp
@@ +1687,5 @@
>     NS_WARNING("Not passing in an aXPAccessible");
>     return NULL;
>    }
>  
> +  IAccessible *msaaAccessible = nsnull;

nit: IAccessible* msaaAccessible = nsnull;

::: accessible/src/msaa/nsHTMLWin32ObjectAccessible.cpp
@@ +118,3 @@
>  
> +NS_IMETHODIMP 
> +nsHTMLWin32ObjectAccessible::GetNativeInterface(void** aOutAccessible)

aOutAccessible -> aNativeAccessible

@@ +121,3 @@
>  {
> +    if (mHwnd) {	
> +       ::AccessibleObjectFromWindow(reinterpret_cast<HWND>(mHwnd),		

use static_cast

::: accessible/src/msaa/nsHTMLWin32ObjectAccessible.h
@@ +89,5 @@
>    virtual ~nsHTMLWin32ObjectAccessible() {}
>  
>    NS_DECL_ISUPPORTS_INHERITED
> +
> +  NS_OVERRIDE NS_IMETHOD GetNativeInterface(void** aOutAccessible);

aNativeAccessible

Comment 15

[Andrew Quartey [:drexler]]

Attachment: Patch v6

Changes per last review comments. Thanks for review.

Comment 16

[alexander :surkov (:asurkov)]

Comment on attachment 595949 [details] [diff] [review]
Patch v6

there's few nits remaining but I'll fix them before landing

Comment 17

[Andrew Quartey [:drexler]]

Sure.

Comment 18

[alexander :surkov (:asurkov)]

inbound land https://hg.mozilla.org/integration/mozilla-inbound/rev/742ae4efad72

Comment 19

[alexander :surkov (:asurkov)]

https://hg.mozilla.org/mozilla-central/rev/742ae4efad72