1454783 - Remove non-android presenters

Status: RESOLVED FIXED

(Core :: Disability Access APIs, enhancement)

Description

[Eitan Isaacson [:eeejay]]

No need for them anymore.

Comment 1

[Eitan Isaacson [:eeejay]]

Attachment: Remove non-Android presenters in AccessFu. r?yzen

Comment 2

[Eitan Isaacson [:eeejay]]

Attachment: Remove non-Android presenters in AccessFu. r?yzen

Removed CSS file we don't need now.

Comment 3

[Eitan Isaacson [:eeejay]]

Attachment: Remove non-Android presenters in AccessFu. r?yzen

Oops, somehow I removed the 'r?' flag earlier, and was wondering why I never heard back. Here is a rebased version of the patch.

Comment 4

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 8970598 [details] [diff] [review]
Remove non-Android presenters in AccessFu. r?yzen

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

thanks, added comments and asuming eslint passes

::: accessible/jsat/AccessFu.jsm
@@ +176,5 @@
> +    }
> +
> +    for (let evt of aPresentationData) {
> +      Utils.win.WindowEventDispatcher.sendRequest(
> +        Object.assign({}, evt, { type: "GeckoView:AccessibilityEvent" }));

nit:
{
  ...evt,
  type: "GeckoView:AccessibilityEvent"
}

::: accessible/jsat/Presentation.jsm
@@ +31,5 @@
> +const ANDROID_VIEW_SCROLLED = 0x1000;
> +const ANDROID_VIEW_TEXT_SELECTION_CHANGED = 0x2000;
> +const ANDROID_ANNOUNCEMENT = 0x4000;
> +const ANDROID_VIEW_ACCESSIBILITY_FOCUSED = 0x8000;
> +const ANDROID_VIEW_TEXT_TRAVERSED_AT_MOVEMENT_GRANULARITY = 0x20000;

For these, should we keep them in an object:

const ANDROID_EVENTS = {
  ANDROID_VIEW_CLICKED: 0x01,
  ...
};

In case we would need to export/import somewhere.

::: accessible/tests/mochitest/jsat/test_content_text.html
@@ +276,5 @@
>  
>        const KEYBOARD_ECHO_SETTING = "accessibility.accessfu.keyboard_echo";
>        function typeKey(key) {
> +        let func = function() { synthesizeKey(key, {}, currentTabWindow()); };
> +        func.toString = () => `typeKey('${key}')`;

can't we just use name property?

::: accessible/tests/mochitest/jsat/test_live_regions.html
@@ +12,5 @@
>    <script type="application/javascript"
>            src="./jsatcommon.js"></script>
>    <script type="application/javascript">
>  
> +    const ANDROID_ANNOUNCEMENT = 0x4000;

Yeah, this is where we are hardcoding stuff, the constants from Presentation.jsm (or if moved somewhere else) should be used here directly.

@@ +65,5 @@
> +        "addedCount": "hidden I will be hidden".length,
> +        "removedCount": 0,
> +        "fromIndex": 0,
> +        // "options": {
> +        //   "enqueue": true

wasn't this a b2g thing? should we remove it?

Comment 5

[Eitan Isaacson [:eeejay]]

Attachment: Remove non-Android presenters in AccessFu.

Comment 6

[Eitan Isaacson [:eeejay]]

(In reply to Yura Zenevich [:yzen] from comment #4)
> Comment on attachment 8970598 [details] [diff] [review]
> Remove non-Android presenters in AccessFu. r?yzen
> 
> Review of attachment 8970598 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> thanks, added comments and asuming eslint passes
> 
> ::: accessible/jsat/AccessFu.jsm
> @@ +176,5 @@
> > +    }
> > +
> > +    for (let evt of aPresentationData) {
> > +      Utils.win.WindowEventDispatcher.sendRequest(
> > +        Object.assign({}, evt, { type: "GeckoView:AccessibilityEvent" }));
> 
> nit:
> {
>   ...evt,
>   type: "GeckoView:AccessibilityEvent"
> }
> 

Very cool syntax. Added.

> ::: accessible/jsat/Presentation.jsm
> @@ +31,5 @@
> > +const ANDROID_VIEW_SCROLLED = 0x1000;
> > +const ANDROID_VIEW_TEXT_SELECTION_CHANGED = 0x2000;
> > +const ANDROID_ANNOUNCEMENT = 0x4000;
> > +const ANDROID_VIEW_ACCESSIBILITY_FOCUSED = 0x8000;
> > +const ANDROID_VIEW_TEXT_TRAVERSED_AT_MOVEMENT_GRANULARITY = 0x20000;
> 
> For these, should we keep them in an object:
> 
> const ANDROID_EVENTS = {
>   ANDROID_VIEW_CLICKED: 0x01,
>   ...
> };
> 
> In case we would need to export/import somewhere.
> 

Good idea, I put the in Constants.jsm

> ::: accessible/tests/mochitest/jsat/test_content_text.html
> @@ +276,5 @@
> >  
> >        const KEYBOARD_ECHO_SETTING = "accessibility.accessfu.keyboard_echo";
> >        function typeKey(key) {
> > +        let func = function() { synthesizeKey(key, {}, currentTabWindow()); };
> > +        func.toString = () => `typeKey('${key}')`;
> 
> can't we just use name property?
> 

We don't get the argument with the name property.


> ::: accessible/tests/mochitest/jsat/test_live_regions.html
> @@ +12,5 @@
> >    <script type="application/javascript"
> >            src="./jsatcommon.js"></script>
> >    <script type="application/javascript">
> >  
> > +    const ANDROID_ANNOUNCEMENT = 0x4000;
> 
> Yeah, this is where we are hardcoding stuff, the constants from
> Presentation.jsm (or if moved somewhere else) should be used here directly.
> 

Done

> @@ +65,5 @@
> > +        "addedCount": "hidden I will be hidden".length,
> > +        "removedCount": 0,
> > +        "fromIndex": 0,
> > +        // "options": {
> > +        //   "enqueue": true
> 
> wasn't this a b2g thing? should we remove it?

Keeping it as a comment for when we reintroduce live regions correctly.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b92a5613a631
Remove non-Android presenters in AccessFu. r=yzen

Comment 8

[Raul Gurzau (:RaulG)]

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