Status

()

RESOLVED FIXED
8 years ago
6 years ago

People

(Reporter: tbsaunde, Assigned: tbsaunde)

Tracking

(Blocks: 1 bug)

unspecified
mozilla8
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Comment hidden (empty)
Blocks: 389800
(Assignee)

Comment 1

7 years ago
Created attachment 548757 [details] [diff] [review]
part1
Attachment #548757 - Flags: review?(surkov.alexander)
Comment on attachment 548757 [details] [diff] [review]
part1

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

r=me

::: accessible/src/base/Makefile.in
@@ +86,5 @@
>    nsAccessibilityService.h \
>    nsAccessible.h \
>    nsAccessNode.h \
>    nsARIAMap.h \
>  	States.h \

remove it from exports

@@ +93,5 @@
> +EXPORTS_NAMESPACES = mozilla/a11y
> +
> +EXPORTS_mozilla/a11y = \
> +	States.h \
> +	$(NULL)

nit: use spaces instead tabs

::: accessible/src/base/States.h
@@ +42,5 @@
>  
>  #include <prtypes.h>
>  
> +namespace mozilla {
> +  namespace a11y {

nit: no indent

@@ +306,5 @@
>     */
>    const PRUint64 EXPANDABLE = ((PRUint64) 0x1) << 46;
>  }
> +}
> +}

} // namespace states
} // namespace a11y
} // namespace mozilla

::: accessible/src/base/nsAccessible.h
@@ +444,5 @@
>      // XXX In order to implement this we would need to follow every link
>      // Perhaps we can get information about invalid links from the cache
>      // In the mean time authors can use role="link" aria-invalid="true"
>      // to force it for links they internally know to be invalid
> +    return (0 == (State() & mozilla::a11y::states::INVALID));

typedef mozilla::a11y::states to states inside private section of the class

::: accessible/src/mac/nsAccessibleWrap.h
@@ +85,5 @@
>      
>      PRInt32 GetUnignoredChildCount(PRBool aDeepCount);
>      
>      PRBool HasPopup () {
> +      return (NativeState() & mozilla::a11y::states::HASPOPUP);

if you typedefed then you can use states namespace directly
Attachment #548757 - Flags: review?(surkov.alexander) → review+
Assignee: nobody → trev.saunders
anything else to be done here? other exported things (except a11yGeneric.h) are prefixed by 'ns' namespace what makes them allowed globals. As far as I know a11y folders aren't included into other modules so probably no win to namespace things like filters or AccIterator. Should we keep exported headers in public folder?
Backed out from mozilla-inbound because the patch doesn't compile on Windows.
e:/builds/moz2_slave/m-in-w32/build/accessible/src/msaa/nsHTMLWin32ObjectAccessible.cpp(80) : error C2653: 'states' : is not a class or namespace name

e:/builds/moz2_slave/m-in-w32/build/accessible/src/msaa/nsHTMLWin32ObjectAccessible.cpp(80) : error C2065: 'UNAVAILABLE' : undeclared identifier

Trevor, you forgot to do using namespace mozilla::a11y.
http://hg.mozilla.org/mozilla-central/rev/cc7357dfd1e2
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
Blocks: 763145
No longer blocks: 389800
You need to log in before you can comment on or make changes to this bug.