The default bug view has changed. See this FAQ.

put ApplicationAccessible class into mozilla::a11y namespace

RESOLVED FIXED in mozilla15

Status

()

Core
Disability Access APIs
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: surkov, Assigned: capella)

Tracking

(Blocks: 1 bug)

unspecified
mozilla15
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][mentor=eitan@monotonous.org][lang=c++])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
was missed in bug 745740
(Assignee)

Updated

5 years ago
Assignee: nobody → markcapella
Status: NEW → ASSIGNED
(Assignee)

Updated

5 years ago
Depends on: 750295
(Assignee)

Updated

5 years ago
Depends on: 750216
No longer depends on: 750295
(Assignee)

Updated

5 years ago
Depends on: 750295
(Assignee)

Comment 1

5 years ago
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."
(Assignee)

Comment 2

5 years ago
Created attachment 622682 [details] [diff] [review]
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
Attachment #622682 - Flags: feedback?(eitan)
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?
Attachment #622682 - Flags: feedback?(eitan) → feedback?(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"
Attachment #622682 - Flags: feedback?(hub) → feedback+
(Reporter)

Comment 5

5 years ago
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
(Assignee)

Comment 6

5 years ago
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 :)
(Assignee)

Comment 7

5 years ago
Created attachment 623417 [details] [diff] [review]
Patch (v2)

Test of nits addressed ... builds and tests local to my WIN machine.
Attachment #622682 - Attachment is obsolete: true
Attachment #623417 - Flags: review?(surkov.alexander)
(Reporter)

Updated

5 years ago
Attachment #623417 - Flags: review?(surkov.alexander) → review+
(Assignee)

Comment 8

5 years ago
TRY test, pushing to inbound next

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

Comment 9

5 years ago
fingers crossed ...

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

Updated

5 years ago
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/c02ce3b34b49
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.