1387507 - Remove a11y e10s app runner disabling code in 57

Status: RESOLVED FIXED

(Core :: Disability Access APIs, enhancement, P1)

Description

[Jim Mathies [:jimm]]

This code needs to go away in 57.

http://searchfox.org/mozilla-central/rev/30a47c4339bd397b937abdb2305f99d3bb537ba6/toolkit/xre/nsAppRunner.cpp#5045

Comment 1

[David Bolter [:davidb] (NeedInfo me for attention)]

There may be some related tests that we should unblock as e10s supports rides up beta and release?
http://searchfox.org/mozilla-central/search?q=RELEASE_OR_BETA&path=accessible

Comment 2

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

(In reply to David Bolter [:davidb] from comment #1)
> There may be some related tests that we should unblock as e10s supports
> rides up beta and release?
> http://searchfox.org/mozilla-central/search?q=RELEASE_OR_BETA&path=accessible

I would double check with Eitan as he's the author, but I think you're right.

Comment 3

[Eitan Isaacson [:eeejay]]

Yeah, those no longer need to be skipped.

Comment 4

[(No longer employed by Mozilla) Aaron Klotz]

(In reply to Jim Mathies [:jimm] from comment #0)
> This code needs to go away in 57.
> 
> http://searchfox.org/mozilla-central/rev/
> 30a47c4339bd397b937abdb2305f99d3bb537ba6/toolkit/xre/nsAppRunner.cpp#5045

There are two additional patches from bug 1371051 that should be reverted as well.

Comment 5

[Jim Mathies [:jimm]]

Attachment: remove a11y e10s checks and prefs v.1

Comment 6

[Jim Mathies [:jimm]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2486a386c4d4ec3c3c66579ce0faf4031321cf9b

Comment 7

[Jim Mathies [:jimm]]

Comment on attachment 8893908 [details] [diff] [review]
remove a11y e10s checks and prefs v.1

r? to felipe for browser glue changes

Comment 8

[Jim Mathies [:jimm]]

(In reply to Jim Mathies [:jimm] from comment #7)
> Comment on attachment 8893908 [details] [diff] [review]
> remove a11y e10s checks and prefs v.1
> 
> r? to felipe for browser glue changes

Actually I think you can review all of this felipe!

Comment 9

[Jim Mathies [:jimm]]

Attachment: enable tests

Comment 10

[Eitan Isaacson [:eeejay]]

Comment on attachment 8894559 [details] [diff] [review]
enable tests

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

lgtm

Comment 11

[Jim Mathies [:jimm]]

Attachment: remove a11y e10s checks and prefs v.1

Ok, I think we're ready for this. We might have to hastily re-land this near the end of the cycle if we don't get testing. I'll keep track of the status on that.

Note I'm going to resurrect a little bit of this prompting code in a follow up bug related to older JAWS screen readers. To start though I'm going to clean this out fully.

Comment 12

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8900909 [details] [diff] [review]
remove a11y e10s checks and prefs v.1

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

Great!

Since there's a chance that this needs to be relanded at the end of beta, I think we should leave the strings unused and then file a follow-up to remove them when we're sure they won't be needed again.

::: dom/ipc/ContentParent.cpp
@@ -1317,1 @@
>        Unused << SendActivateA11y(::GetCurrentThreadId(),

uber nit: this line will end up with too much indent if it's not adjusted

@@ -2851,1 @@
>          Unused << SendActivateA11y(::GetCurrentThreadId(),

same

Comment 13

[:Felipe Gomes (needinfo for replies!)]

Ah, and I just noticed, this needs to remove the [e-mail] file too and the references to it in the jar.mn files

Comment 14

[Jim Mathies [:jimm]]

(In reply to :Felipe Gomes (needinfo me!) from comment #12)
> Comment on attachment 8900909 [details] [diff] [review]
> remove a11y e10s checks and prefs v.1
> 
> Review of attachment 8900909 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Great!
> 
> Since there's a chance that this needs to be relanded at the end of beta, I
> think we should leave the strings unused and then file a follow-up to remove
> them when we're sure they won't be needed again.

Will do. I'll create new strings for the prompt in 1385991 as well so these strings remain unchanged.

> 
> ::: dom/ipc/ContentParent.cpp
> @@ -1317,1 @@
> >        Unused << SendActivateA11y(::GetCurrentThreadId(),
> 
> uber nit: this line will end up with too much indent if it's not adjusted
> 
> @@ -2851,1 @@
> >          Unused << SendActivateA11y(::GetCurrentThreadId(),
> 
> same

updated, thanks.

Comment 15

[Jim Mathies [:jimm]]

Attachment: remove a11y e10s checks and prefs v.2

Comment 16

[Jim Mathies [:jimm]]

both patches posted here.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ab66385c2fb5
Remove a11y e10s checks and preferences. r=felipe
https://hg.mozilla.org/integration/mozilla-inbound/rev/e380765f9845
Enable disabled a11y tests. r=eitan

Comment 18

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ccacb40ce6c7
Backed out changeset e380765f9845 
https://hg.mozilla.org/integration/mozilla-inbound/rev/658c8cac1523
Backed out changeset ab66385c2fb5 for failing browser-chrome's browser_all_files_referenced.js: unreferenced file [e-mail]. r=backout on a CLOSED TREE

Comment 19

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for failing browser-chrome's browser_all_files_referenced.js: unreferenced file [e-mail]:

https://hg.mozilla.org/integration/mozilla-inbound/rev/658c8cac152379c3a56d9c17a3bc48efa5e5696c
https://hg.mozilla.org/integration/mozilla-inbound/rev/ccacb40ce6c7a066dea196ecdcfa89e9c468d6a1

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=e380765f98454933f9df4f521c3bf522a4d0c7df&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=127213162&repo=mozilla-inbound

Comment 20

[Jim Mathies [:jimm]]

argh, I guess that image will have to come out temporarily.

Comment 21

[Jim Mathies [:jimm]]

Recent try push, I'm seeing new a11y test failures. Need to sort this out as well before landing.
 
FAIL | accessible/tests/mochitest/events/test_selection.xul | Test timed out. [log…]
FAIL | accessible/tests/mochitest/treeupdate/test_menubutton.xul | Test timed out. [log…] 

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0004894c99723bed99dd27e4fd86840cbb7f05e2

Comment 22

[:Felipe Gomes (needinfo for replies!)]

(In reply to Jim Mathies [:jimm] from comment #20)
> argh, I guess that image will have to come out temporarily.

Maybe you can land bug 1385991 at the same time to avoid that too

Comment 23

[Jim Mathies [:jimm]]

(In reply to Jim Mathies [:jimm] from comment #21)
> Recent try push, I'm seeing new a11y test failures. Need to sort this out as
> well before landing.
>  
> FAIL | accessible/tests/mochitest/events/test_selection.xul | Test timed
> out. [log…]
> FAIL | accessible/tests/mochitest/treeupdate/test_menubutton.xul | Test
> timed out. [log…] 
> 
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=0004894c99723bed99dd27e4fd86840cbb7f05e

Popped off my patch queue, pulled, and pushed just the changes here - clean a11y run. \0/

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1974cdfd8774c50c8e12a792a4429642fd890000

Comment 24

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Jimm mentioned this bug fix will likely land in 57 as per the e10s/a11y plans

Comment 25

[Jim Mathies [:jimm]]

Attachment: remove a11y e10s checks and prefs v.3

- fixed image issue associated with leftover icon

https://treeherder.mozilla.org/#/jobs?repo=try&revision=426152843e19e6afddbbc92c13d01ab4bfecb601

Comment 26

[Jim Mathies [:jimm]]

Attachment: enable tests

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e7a9bb3ff772
Remove a11y e10s checks and preferences. r=felipe
https://hg.mozilla.org/integration/mozilla-inbound/rev/0b442d470556
Enable disabled a11y tests. r=eitan

Comment 28

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/e7a9bb3ff772
https://hg.mozilla.org/mozilla-central/rev/0b442d470556