Closed Bug 1479033 Opened 2 years ago Closed 1 year ago

Introduce accessible/android source directory

Categories

(Core :: Disability Access APIs, enhancement)

Unspecified
Android
enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: eeejay, Assigned: eeejay)

References

Details

Attachments

(1 file, 1 obsolete file)

.. a brave first step.
Blocks: 1479034
Blocks: 1479037
Attachment #8996422 - Flags: review?(surkov.alexander)
Comment on attachment 8996422 [details] [diff] [review]
Introduce Android accessibility directory. r?surkov

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

::: accessible/android/AccessibleWrap.h
@@ +15,5 @@
> +
> +namespace mozilla {
> +namespace a11y {
> +
> +class AccessibleWrap : public Accessible

you expect to fill it up with something, and that's why you can't go with typedef, correct?

::: accessible/android/ApplicationAccessibleWrap.h
@@ +1,1 @@
> +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */

4 -> 2

::: accessible/android/moz.build
@@ +22,5 @@
> +    '/accessible/ipc/other',
> +    '/accessible/xul',
> +    '/dom/base',
> +    '/widget',
> +    '/widget/android',

I would suggest to add those as you go

@@ +25,5 @@
> +    '/widget',
> +    '/widget/android',
> +]
> +
> +FINAL_LIBRARY = 'xul'

are you sure about this one?

::: accessible/other/Platform.cpp
@@ -61,5 @@
>  }
>  
> -#if defined(ANDROID)
> -void
> -a11y::ProxyVirtualCursorChangeEvent(ProxyAccessible*, ProxyAccessible*,

nice
Attachment #8996422 - Flags: review?(surkov.alexander) → review+
OK, can't sit on this forever..

This also has those little unique ID changes we talked about.
Attachment #8996422 - Attachment is obsolete: true
Attachment #9002635 - Flags: review?(jteh)
Comment on attachment 9002635 [details] [diff] [review]
Introduce Android accessibility directory. r?Jamie

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

Yay! :)

Aside from the nits, please amend the commit summary to note that this is the skeleton for the Android native a11y implementation or similar, as it does more than just create a directory. Also, in the commit message body, please mention the use of ids from the content process in the Android layer. Otherwise, I fear "blame" on the UniqueID change will be very confusing years down the track.

::: accessible/android/ARIAGridAccessibleWrap.h
@@ +19,5 @@
> +} // namespace a11y
> +} // namespace mozilla
> +
> +#endif
> +

nit: Extraneous blank line.

::: accessible/android/AccessibleWrap.h
@@ +3,5 @@
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +/* For documentation of the accessibility architecture,
> + * see http://lxr.mozilla.org/seamonkey/source/accessible/accessible-docs.html

nit: The files in which this comment appears seem somewhat arbitrary. Also, the URL is now invalid. I'd just remove this comment.

::: accessible/android/ApplicationAccessibleWrap.h
@@ +17,5 @@
> +} // namespace a11y
> +} // namespace mozilla
> +
> +#endif
> +

nit: Extraneous blank line.

::: accessible/android/DocAccessibleWrap.h
@@ +3,5 @@
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +/* For documentation of the accessibility architecture,
> + * see http://lxr.mozilla.org/seamonkey/source/accessible/accessible-docs.html

See above re URL comment.

::: accessible/android/HTMLTableAccessibleWrap.h
@@ +20,5 @@
> +} // namespace a11y
> +} // namespace mozilla
> +
> +#endif
> +

nit: Extraneous blank line.

::: accessible/android/HyperTextAccessibleWrap.h
@@ +16,5 @@
> +} // namespace a11y
> +} // namespace mozilla
> +
> +#endif
> +

nit: Extraneous blank line.

::: accessible/android/ImageAccessibleWrap.h
@@ +18,5 @@
> +} // namespace a11y
> +} // namespace mozilla
> +
> +#endif
> +

nit: Extraneous blank line.

::: accessible/android/RootAccessibleWrap.h
@@ +3,5 @@
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +/* For documentation of the accessibility architecture,
> + * see http://lxr.mozilla.org/seamonkey/source/accessible/accessible-docs.html

As above re URL comment.
Attachment #9002635 - Flags: review?(jteh) → review+
Pushed by eisaacson@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1b683d101ee9
Introduce Android accessibility directory. r=Jamie
https://hg.mozilla.org/mozilla-central/rev/1b683d101ee9
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.