553174 - expose nsIAccessibleApplication similar to IA2

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

1) we can test the part of IA2 functionality in crossplatform way
2) it might be handy for AT scripts to get application info
3) it's handy for AT scripts to detect this object is application accessible

Comment 1

[David Bolter [:davidb] (NeedInfo me for attention)]

(In reply to comment #0)
> 1) we can test the part of IA2 functionality in crossplatform way

Please explain.

Comment 2

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 3

[alexander :surkov (:asurkov)]

(In reply to comment #1)
> (In reply to comment #0)
> > 1) we can test the part of IA2 functionality in crossplatform way
> 
> Please explain.

We can make sure we expose application name and version by mochitests.

Comment 4

[David Bolter [:davidb] (NeedInfo me for attention)]

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

>+++ b/accessible/public/nsIAccessibleApplication.idl

>+ * The Initial Developer of the Original Code is
>+ * Netscape Communications Corporation.

Mozilla Corporation.

Comment 5

[alexander :surkov (:asurkov)]

(In reply to comment #4)

> Mozilla Corporation.

yep, thaks, copy/paste. Also I removed wrong alert() in test and fixed getApplicationAccessible() in atk/nsApplicationAccessibleWrap.

Comment 6

[David Bolter [:davidb] (NeedInfo me for attention)]

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

r=me with your catches :) and the initial developer fix... and up to you:

(There was some code here I hadn't looked at before)

>+interface nsIAccessibleApplication : nsISupports
>+{
>+  
>+  /**
>+   * Return the application name.

nit: I guess "Returns"

>-    nsRefPtr<nsApplicationAccessibleWrap> root =
>+    nsApplicationAccessibl applicationAcc =

;)

>+
>+NS_IMETHODIMP
>+nsApplicationAccessible::GetAppName(nsAString& aName)

Should we have GetName just call GetAppName instead of what it does now?

>+NS_IMETHODIMP
>+nsApplicationAccessible::GetPlatformName(nsAString& aName)
>+{
>+  aName.AssignLiteral("Gecko");
>+  return NS_OK;
>+}

Maybe ping bsmedberg for the right name here? I'm fine with gecko.

> 
> STDMETHODIMP
> nsApplicationAccessibleWrap::get_toolkitName(BSTR *aName)
> {
> __try {
>-  *aName = ::SysAllocString(L"Gecko");

Ah so we have precedent. Hmmm I guess we shouldn't change it then (?)

>+  *aName = ::SysAllocStringLen(name.get(), name.Length());

First time looking at this code in detail... I guess name.get() returns a OLECHAR FAR* ?

>+  alert(Components.interfaces.nsIAccessibleApplication);

;)

Comment 7

[Marco Zehe (:MarcoZ)]

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

>+   * Return the application name.

Please be consistent, all the others read:

>+   * Returns the application version.

So the first one should be "returns" as well.


>+  alert(Components.interfaces.nsIAccessibleApplication);

Please remove or we'll have hanging tests. ;)

r=me with the above fixed.

Comment 8

[alexander :surkov (:asurkov)]

(In reply to comment #6)

> Should we have GetName just call GetAppName instead of what it does now?

I'll check it it's the same.

> Maybe ping bsmedberg for the right name here? I'm fine with gecko.

Before to change we need to check if anybody uses it. The "gecko" word is not young :)

> Ah so we have precedent. Hmmm I guess we shouldn't change it then (?)

yep.

> >+  *aName = ::SysAllocStringLen(name.get(), name.Length());
> 
> First time looking at this code in detail... I guess name.get() returns a
> OLECHAR FAR* ?

it should be :)

Comment 9

[alexander :surkov (:asurkov)]

(In reply to comment #8)
> (In reply to comment #6)
> 
> > Should we have GetName just call GetAppName instead of what it does now?
> 
> I'll check it it's the same.

Yep, they are different GetName() returns brand name (Minefield), GetAppName() returns product name (Firefox). It might be they should be the same. But not in this bug.

Comment 10

[David Bolter [:davidb] (NeedInfo me for attention)]

(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #6)
> > 
> > > Should we have GetName just call GetAppName instead of what it does now?
> > 
> > I'll check it it's the same.
> 
> Yep, they are different GetName() returns brand name (Minefield), GetAppName()
> returns product name (Firefox). It might be they should be the same. But not in
> this bug.

Yep, ok.

Comment 11

[alexander :surkov (:asurkov)]

landed on 1.9.3 - http://hg.mozilla.org/mozilla-central/rev/0552741c6bd7