Closed
Bug 1387507
Opened 7 years ago
Closed 7 years ago
Remove a11y e10s app runner disabling code in 57
Categories
(Core :: Disability Access APIs, enhancement, P1)
Core
Disability Access APIs
Tracking
()
RESOLVED
FIXED
mozilla57
People
(Reporter: jimm, Assigned: jimm)
References
Details
(Whiteboard: aes+)
Attachments
(2 files, 4 obsolete files)
23.38 KB,
patch
|
Details | Diff | Splinter Review | |
3.15 KB,
patch
|
Details | Diff | Splinter Review |
This code needs to go away in 57. http://searchfox.org/mozilla-central/rev/30a47c4339bd397b937abdb2305f99d3bb537ba6/toolkit/xre/nsAppRunner.cpp#5045
Comment 1•7 years ago
|
||
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
Flags: needinfo?(yzenevich)
Comment 2•7 years ago
|
||
(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.
Flags: needinfo?(yzenevich) → needinfo?(eitan)
Comment 4•7 years ago
|
||
(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.
Assignee | ||
Comment 5•7 years ago
|
||
Assignee | ||
Comment 6•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2486a386c4d4ec3c3c66579ce0faf4031321cf9b
Assignee | ||
Updated•7 years ago
|
Blocks: post-57-api-changes
Assignee | ||
Comment 7•7 years ago
|
||
Comment on attachment 8893908 [details] [diff] [review] remove a11y e10s checks and prefs v.1 r? to felipe for browser glue changes
Attachment #8893908 -
Flags: review?(felipc)
Assignee | ||
Comment 8•7 years ago
|
||
(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!
Assignee | ||
Comment 9•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #8894559 -
Flags: review?(eitan)
Comment 10•7 years ago
|
||
Comment on attachment 8894559 [details] [diff] [review] enable tests Review of attachment 8894559 [details] [diff] [review]: ----------------------------------------------------------------- lgtm
Attachment #8894559 -
Flags: review?(eitan) → review+
Assignee | ||
Updated•7 years ago
|
Attachment #8893908 -
Flags: review?(felipc)
Assignee | ||
Updated•7 years ago
|
No longer blocks: post-57-api-changes
Assignee | ||
Updated•7 years ago
|
Assignee | ||
Comment 11•7 years ago
|
||
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.
Attachment #8893908 -
Attachment is obsolete: true
Attachment #8900909 -
Flags: review?(felipc)
Comment 12•7 years ago
|
||
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
Attachment #8900909 -
Flags: review?(felipc) → review+
Comment 13•7 years ago
|
||
Ah, and I just noticed, this needs to remove the e10s-64@2x.png file too and the references to it in the jar.mn files
Assignee | ||
Comment 14•7 years ago
|
||
(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.
Assignee | ||
Comment 15•7 years ago
|
||
Attachment #8900909 -
Attachment is obsolete: true
Attachment #8902749 -
Flags: review+
Comment 17•7 years ago
|
||
Pushed by ryanvm@gmail.com: 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
Keywords: checkin-needed
Comment 18•7 years ago
|
||
Backout by archaeopteryx@coole-files.de: 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 e10s-64@2x.png. r=backout on a CLOSED TREE
Comment 19•7 years ago
|
||
Backed out for failing browser-chrome's browser_all_files_referenced.js: unreferenced file e10s-64@2x.png: 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
Flags: needinfo?(jmathies)
Assignee | ||
Comment 20•7 years ago
|
||
argh, I guess that image will have to come out temporarily.
Flags: needinfo?(jmathies)
Assignee | ||
Comment 21•7 years ago
|
||
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•7 years ago
|
||
(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
Assignee | ||
Comment 23•7 years ago
|
||
(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
Jimm mentioned this bug fix will likely land in 57 as per the e10s/a11y plans
tracking-firefox57:
--- → +
Updated•7 years ago
|
Priority: -- → P1
Assignee | ||
Comment 25•7 years ago
|
||
- fixed image issue associated with leftover icon https://treeherder.mozilla.org/#/jobs?repo=try&revision=426152843e19e6afddbbc92c13d01ab4bfecb601
Attachment #8894559 -
Attachment is obsolete: true
Attachment #8902749 -
Attachment is obsolete: true
Assignee | ||
Comment 26•7 years ago
|
||
Comment 27•7 years ago
|
||
Pushed by jmathies@mozilla.com: 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•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e7a9bb3ff772 https://hg.mozilla.org/mozilla-central/rev/0b442d470556
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in
before you can comment on or make changes to this bug.
Description
•