748719 - put ApplicationAccessible class into mozilla::a11y namespace

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

was missed in bug 745740

Comment 1

[Mark Capella [:capella]]

FYI, I'll remove accessible-docs.html during this patch instead of a new bug as mentioned in bug 748724 comment 13 :

"btw, would you mind to file a bug to remove accessible-docs.html from sources, it's out of date and all docs should be on devmo."

Comment 2

[Mark Capella [:capella]]

Attachment: Patch (v1)

Asking mentor for initial feedback ...

Did a push to try ... I build on WIN but this affects builds cross platform ... not sure why just the OS X 10.7 opt went red ...
https://tbpl.mozilla.org/?tree=Try&rev=7df18c639c92

Comment 3

[Eitan Isaacson [:eeejay]]

Comment on attachment 622682 [details] [diff] [review]
Patch (v1)

It is hard to say why this fails to build on mac. Hub, would you mind trying?

Comment 4

[Hubert Figuiere [:hub]]

Comment on attachment 622682 [details] [diff] [review]
Patch (v1)

it got restarted and worked. I don't know what happened. tbpl says "0 jobs failing"

Comment 5

[alexander :surkov (:asurkov)]

Comment on attachment 622682 [details] [diff] [review]
Patch (v1)

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

::: accessible/src/atk/ApplicationAccessibleWrap.h
@@ +38,5 @@
>   *
>   * ***** END LICENSE BLOCK ***** */
>  
> +#ifndef mozilla_a11y_Application_Accessible_Wrap_h__
> +#define mozilla_a11y_Application_Accessible_Wrap_h__

nit: mozilla_a11y_ApplicationAccessibleWrap_h__

@@ +66,2 @@
>  
> +  // return the atk object for app root accessible

please change it to 
/**
 * Return the atk object for app root accessible.
 */

::: accessible/src/base/nsAccessNode.cpp
@@ +60,5 @@
>  /* For documentation of the accessibility architecture, 
>   * see http://lxr.mozilla.org/seamonkey/source/accessible/accessible-docs.html
>   */
>  
> +mozilla::a11y::ApplicationAccessible* nsAccessNode::gApplicationAccessible = nsnull;

do you really need mozilla::a11y:: because there's using namespace mozilla::a11y?

::: accessible/src/generic/ApplicationAccessible.h
@@ +40,5 @@
>   *
>   * ***** END LICENSE BLOCK ***** */
>  
> +#ifndef mozilla_a11y_Application_Accessible_h__
> +#define mozilla_a11y_Application_Accessible_h__

same here: no '_' between Application and Accessible and in other files

::: accessible/src/msaa/nsAccessNodeWrap.cpp
@@ +173,5 @@
>    }
>  
>    // Can get to IAccessibleApplication from any node via QS
>    if (iid == IID_IAccessibleApplication) {
> +    mozilla::a11y::ApplicationAccessible* applicationAcc = 

you don't need mozilla::a11y, right?

@@ +175,5 @@
>    // Can get to IAccessibleApplication from any node via QS
>    if (iid == IID_IAccessibleApplication) {
> +    mozilla::a11y::ApplicationAccessible* applicationAcc = 
> +      reinterpret_cast<mozilla::a11y::ApplicationAccessible*>(GetApplicationAccessible());
> +      GetApplicationAccessible();

you forgot to remove GetApplicationAccessible.
I'm curious why do you need reinterpret_cast and casting at all

Comment 6

[Mark Capella [:capella]]

FYI re: you forgot to remove GetApplicationAccessible.
I'm curious why do you need reinterpret_cast and casting at all

I put this in while resolving (what turned out to be) unrelated build errors and forgot to remove it ... thanks for the tip :)

Comment 7

[Mark Capella [:capella]]

Attachment: Patch (v2)

Test of nits addressed ... builds and tests local to my WIN machine.

Comment 8

[Mark Capella [:capella]]

TRY test, pushing to inbound next

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

Comment 9

[Mark Capella [:capella]]

fingers crossed ...

https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=c02ce3b34b49

Comment 10

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/c02ce3b34b49