Remove non-android presenters

RESOLVED FIXED in Firefox 61

Status

()

enhancement
RESOLVED FIXED
Last year
Last year

People

(Reporter: eeejay, Assigned: eeejay)

Tracking

unspecified
mozilla61
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox61 fixed)

Details

Attachments

(1 attachment, 3 obsolete attachments)

No need for them anymore.
Attachment #8968663 - Flags: review?(yzenevich)
Removed CSS file we don't need now.
Attachment #8968663 - Attachment is obsolete: true
Attachment #8968663 - Flags: review?(yzenevich)
Oops, somehow I removed the 'r?' flag earlier, and was wondering why I never heard back. Here is a rebased version of the patch.
Attachment #8970598 - Flags: review?(yzenevich)
Attachment #8968746 - Attachment is obsolete: true
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?
Attachment #8970598 - Flags: review?(yzenevich) → review+
Attachment #8970598 - Attachment is obsolete: true
(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.
Keywords: checkin-needed
Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b92a5613a631
Remove non-Android presenters in AccessFu. r=yzen
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b92a5613a631
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.