Closed Bug 1602075 Opened 5 years ago Closed 4 years ago

Switch to lazily turning a11y services on/off when using accessibility panel.

Categories

(DevTools :: Accessibility Tools, task, P2)

Product:
DevTools
Component:
Accessibility Tools
Type:
task

Tracking

(firefox78 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 78
Tracking Flags:
Tracking Status
firefox78 --- fixed

People

(Reporter: yzen, Assigned: yzen)

Details

(Keywords: dev-doc-complete)

Attachments

(6 files)

Bug 1602075 - add an accessibility-panel-auto-init feature to control the panel auto enabling functionality. r=mythmon,mtigley
47 bytes, text/x-phabricator-request
pascalc
: approval-mozilla-beta-
Details | Review
Bug 1602075 - handle can-be-disabled and can-be-disabled events in the MainFrame component instead of the Toolbar and Description respectively. r=mtigley
47 bytes, text/x-phabricator-request
pascalc
: approval-mozilla-beta-
Details | Review
Bug 1602075 - make enable/disable panel UI conditional on the accessibility-panel-auto-init feature. r=mtigley
47 bytes, text/x-phabricator-request
pascalc
: approval-mozilla-beta-
Details | Review
Bug 1602075 - only highlight the accessibility tab when the auto init feature is not enabled. r=mtigley
47 bytes, text/x-phabricator-request
pascalc
: approval-mozilla-beta-
Details | Review
Bug 1602075 - display Inspect Accessibility Properties menu item if devtools.accessibility.auto-init.enabled pref is set to true. r=gijs
47 bytes, text/x-phabricator-request
pascalc
: approval-mozilla-beta-
Details | Review
Bug 1602075 - display enabled Inspect Accessibility Properties menu item in inspector if devtools.accessibility.auto-init.enabled pref is set to true. r=gl
47 bytes, text/x-phabricator-request
pascalc
: approval-mozilla-beta-
Details | Review

We'd like to experiment with turning the panel on (on open or accessibility properties inspection from Browser or Inspector). This should help with a11y panel discoverability and potentially might not have any negative impact on retention, etc.

Originally, the panel was disabled by default because it was released soon after Fx57 Quantum when transition to multi-process resulted visible performance regressions for accessibility services in the browser. Since then there was a number of improvements that might've helped with mitigating the performance hit on the browser. We would still disable accessibility services when the toolbox closes.

Assignee: nobody → yzenevich

Have we got telemetry or other measures in place to see how these changes work out in practice, in terms of perf impact and/or how often this triggers?

Flags: needinfo?(yzenevich)

(In reply to :Gijs (he/him) from comment #7)

Have we got telemetry or other measures in place to see how these changes work out in practice, in terms of perf impact and/or how often this triggers?

We have the telemetry for measuring impact on usage and navigation to devtools. In terms of performance, it's the matter of 1 additional perf query when building the menu. In terms of enabling a11y panel, this is what we are going to run the experiment for. We will try to determine if there's any negative effect on devtools userbase retention and usage.

Flags: needinfo?(yzenevich)
Pushed by yura.zenevich@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/7d568f0fa1e4
add an accessibility-panel-auto-init feature to control the panel auto enabling functionality. r=mythmon,mtigley
https://hg.mozilla.org/integration/autoland/rev/3aa7f4f31934
handle can-be-disabled and can-be-disabled events in the MainFrame component instead of the Toolbar and Description respectively. r=mtigley
https://hg.mozilla.org/integration/autoland/rev/e1e570497527
make enable/disable panel UI conditional on the accessibility-panel-auto-init feature. r=mtigley
https://hg.mozilla.org/integration/autoland/rev/fbb30bd09d66
only highlight the accessibility tab when the auto init feature is not enabled. r=mtigley
https://hg.mozilla.org/integration/autoland/rev/51f01135d82f
display Inspect Accessibility Properties menu item if devtools.accessibility.auto-init.enabled pref is set to true. r=Gijs
https://hg.mozilla.org/integration/autoland/rev/cf06e30be068
display Inspect Accessibility Properties menu item in inspector if devtools.accessibility.auto-init.enabled pref is set to true. r=gl
Priority: -- → P2

Backed out for failures caused by markup-context-menu.js

backout: https://hg.mozilla.org/integration/autoland/rev/024226bfc5541965ddff9377ec7fc040b4765e14

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=cf06e30be06862b9855a0fdb043856cf24d989ca&searchStr=devtools&selectedJob=299033761

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=299033689&repo=autoland&lineNumber=1972

[task 2020-04-23T15:11:44.047Z] 15:11:44 INFO - TEST-UNEXPECTED-FAIL | devtools/client/inspector/test/browser_inspector_addNode_01.js | A promise chain failed to handle a rejection: can't access property "emit", this.inspector is null - stack: _updateA11YMenuItem@resource://devtools/client/inspector/markup/markup-context-menu.js:986:5
[task 2020-04-23T15:11:44.047Z] 15:11:44 INFO - async*_buildA11YMenuItem@resource://devtools/client/inspector/markup/markup-context-menu.js:405:12
[task 2020-04-23T15:11:44.047Z] 15:11:44 INFO - _openMenu@resource://devtools/client/inspector/markup/markup-context-menu.js:850:10
[task 2020-04-23T15:11:44.047Z] 15:11:44 INFO - openContextMenuAndGetAllItems@chrome://mochitests/content/browser/devtools/client/inspector/test/shared-head.js:700:45
[task 2020-04-23T15:11:44.047Z] 15:11:44 INFO - @chrome://mochitests/content/browser/devtools/client/inspector/test/browser_inspector_addNode_01.js:14:53
[task 2020-04-23T15:11:44.047Z] 15:11:44 INFO - Async*Tester_execTest/<@chrome://mochikit/content/browser-test.js:1039:34
[task 2020-04-23T15:11:44.047Z] 15:11:44 INFO - Tester_execTest@chrome://mochikit/content/browser-test.js:1074:11
[task 2020-04-23T15:11:44.047Z] 15:11:44 INFO - nextTest/<@chrome://mochikit/content/browser-test.js:904:14
[task 2020-04-23T15:11:44.047Z] 15:11:44 INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:918:23
[task 2020-04-23T15:11:44.047Z] 15:11:44 INFO - Rejection date: Thu Apr 23 2020 15:11:43 GMT+0000 (Coordinated Universal Time) - false == true - JS frame :: resource://testing-common/PromiseTestUtils.jsm :: assertNoUncaughtRejections :: line 265
[task 2020-04-23T15:11:44.047Z] 15:11:44 INFO - Stack trace:
[task 2020-04-23T15:11:44.048Z] 15:11:44 INFO - resource://testing-common/PromiseTestUtils.jsm:assertNoUncaughtRejections:265
[task 2020-04-23T15:11:44.048Z] 15:11:44 INFO - chrome://mochikit/content/browser-test.js:nextTest:603
[task 2020-04-23T15:11:44.048Z] 15:11:44 INFO - chrome://mochikit/content/browser-test.js:testScope/test_finish/<:1434
[task 2020-04-23T15:11:44.048Z] 15:11:44 INFO - chrome://mochikit/content/browser-test.js:run:1349
[task 2020-04-23T15:11:44.048Z] 15:11:44 INFO - GECKO(1823) | MEMORY STAT vsizeMaxContiguous not supported in this build configuration.
[task 2020-04-23T15:11:44.048Z] 15:11:44 INFO - GECKO(1823) | MEMORY STAT heapAllocated not supported in this build configuration.
[task 2020-04-23T15:11:44.049Z] 15:11:44 INFO - GECKO(1823) | MEMORY STAT | vsize 20975307MB | residentFast 1109MB
[task 2020-04-23T15:11:44.049Z] 15:11:44 INFO - TEST-OK | devtools/client/inspector/test/browser_inspector_addNode_01.js | took 6418ms

Flags: needinfo?(yzenevich)
Attachment #9141741 - Attachment description: Bug 1602075 - add an accessibility-panel-auto-init feature to control the panel auto enabling functionality. r=mythmon → Bug 1602075 - add an accessibility-panel-auto-init feature to control the panel auto enabling functionality. r=mythmon, mtigley
Attachment #9141746 - Attachment description: Bug 1602075 - display Inspect Accessibility Properties menu item in inspector if devtools.accessibility.auto-init.enabled pref is set to true. r=gl → Bug 1602075 - display enabled Inspect Accessibility Properties menu item in inspector if devtools.accessibility.auto-init.enabled pref is set to true. r=gl
Pushed by yura.zenevich@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/98a4b4e8fda3
add an accessibility-panel-auto-init feature to control the panel auto enabling functionality. r=mythmon,mtigley
https://hg.mozilla.org/integration/autoland/rev/2c19213df4d7
handle can-be-disabled and can-be-disabled events in the MainFrame component instead of the Toolbar and Description respectively. r=mtigley
https://hg.mozilla.org/integration/autoland/rev/418dbded9752
make enable/disable panel UI conditional on the accessibility-panel-auto-init feature. r=mtigley
https://hg.mozilla.org/integration/autoland/rev/0aa8307788b6
only highlight the accessibility tab when the auto init feature is not enabled. r=mtigley
https://hg.mozilla.org/integration/autoland/rev/1406b19b25e9
display Inspect Accessibility Properties menu item if devtools.accessibility.auto-init.enabled pref is set to true. r=Gijs
https://hg.mozilla.org/integration/autoland/rev/2b11feb2473d
display enabled Inspect Accessibility Properties menu item in inspector if devtools.accessibility.auto-init.enabled pref is set to true. r=gl
Backout by apavel@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2b0e2483e2ea
Backed out 7 changesets (bug 1602075, bug 1551574) for failing multiple dt e.g. browser_accessibility_context_menu_inspector.js on a CLOSED TREE
Flags: needinfo?(yzenevich)
Keywords: leave-open
Pushed by yura.zenevich@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/69d5ff6275a9
add an accessibility-panel-auto-init feature to control the panel auto enabling functionality. r=mythmon,mtigley
https://hg.mozilla.org/integration/autoland/rev/f9017a10e8df
handle can-be-disabled and can-be-disabled events in the MainFrame component instead of the Toolbar and Description respectively. r=mtigley
https://hg.mozilla.org/integration/autoland/rev/d6c51e1b3fb3
make enable/disable panel UI conditional on the accessibility-panel-auto-init feature. r=mtigley
Attachment #9141741 - Attachment description: Bug 1602075 - add an accessibility-panel-auto-init feature to control the panel auto enabling functionality. r=mythmon, mtigley → Bug 1602075 - add an accessibility-panel-auto-init feature to control the panel auto enabling functionality. r=mythmon,mtigley
Pushed by yura.zenevich@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/595a28396874
add an accessibility-panel-auto-init feature to control the panel auto enabling functionality. r=mythmon,mtigley
https://hg.mozilla.org/integration/autoland/rev/fa77554062c7
handle can-be-disabled and can-be-disabled events in the MainFrame component instead of the Toolbar and Description respectively. r=mtigley
https://hg.mozilla.org/integration/autoland/rev/2df583e25890
make enable/disable panel UI conditional on the accessibility-panel-auto-init feature. r=mtigley
Flags: needinfo?(yzenevich)
Pushed by yura.zenevich@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/60d719fdbb52
add an accessibility-panel-auto-init feature to control the panel auto enabling functionality. r=mythmon,mtigley
Pushed by yura.zenevich@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/eb04d196cbf8
handle can-be-disabled and can-be-disabled events in the MainFrame component instead of the Toolbar and Description respectively. r=mtigley
https://hg.mozilla.org/integration/autoland/rev/aaf620bbf3d6
make enable/disable panel UI conditional on the accessibility-panel-auto-init feature. r=mtigley
Status: NEW → ASSIGNED
Keywords: leave-open
Pushed by yura.zenevich@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/9962c44f85cd
only highlight the accessibility tab when the auto init feature is not enabled. r=mtigley
https://hg.mozilla.org/integration/autoland/rev/0f2c4bc49518
display Inspect Accessibility Properties menu item if devtools.accessibility.auto-init.enabled pref is set to true. r=Gijs
https://hg.mozilla.org/integration/autoland/rev/aedc6555b50c
display enabled Inspect Accessibility Properties menu item in inspector if devtools.accessibility.auto-init.enabled pref is set to true. r=gl

https://hg.mozilla.org/mozilla-central/rev/9962c44f85cd
https://hg.mozilla.org/mozilla-central/rev/0f2c4bc49518
https://hg.mozilla.org/mozilla-central/rev/aedc6555b50c

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
status-firefox78: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 78

Comment on attachment 9141741 [details]
Bug 1602075 - add an accessibility-panel-auto-init feature to control the panel auto enabling functionality. r=mythmon,mtigley

Beta/Release Uplift Approval Request

  • User impact if declined: We will not be able to run an experiment in Firefox DevEdition 77. The experiment is intended to see if we can start enabling accessibility panel by default whenever it is opened.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: Bug 1551574
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This should be not risky because any of the functionality that is intended for the experiment is behind the pref. The only way to access it in beta and consequently on release if user discovers and switches the pref (which is unlikely). The population of the experiment where the pref would be flipped is only limited to a fraction of the DevEdition users in the test cohort.
  • String changes made/needed:
Attachment #9141741 - Flags: approval-mozilla-beta?
Attachment #9141742 - Flags: approval-mozilla-beta?
Attachment #9141743 - Flags: approval-mozilla-beta?
Attachment #9141744 - Flags: approval-mozilla-beta?
Attachment #9141745 - Flags: approval-mozilla-beta?
Attachment #9141746 - Flags: approval-mozilla-beta?

Comment on attachment 9141741 [details]
Bug 1602075 - add an accessibility-panel-auto-init feature to control the panel auto enabling functionality. r=mythmon,mtigley

Talked with the team and we should let it ride the train, thanks.

Attachment #9141741 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Attachment #9141742 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Attachment #9141743 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Attachment #9141744 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Attachment #9141746 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Attachment #9141745 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Keywords: dev-doc-needed

:yzen : Is this description accurate? (Checking before I update the MDN docs)

If devtools.accessibility.auto-init.enabled is true, then:

  • Opening the Accessibility Inspector automatically enables accessibility features, with no other action required.
  • A Show Accessibility Properties item is available in the context menu in the DOM and Style Inspector.
  • Clicking this item opens the Accessibility Inspector and displays properties of the element that was right-clicked in the DOM and Style Inspector, with no other action required to enable accessibility features.
  • Switching from the Accessibility Inspector to another tool turns accessibility features off automatically, with no other action required.
Flags: needinfo?(yzenevich)

Hi Janet, Thanks for following up on this!

This is right now running as an informal experiment on DevEdition and Early beta. We enabled the pref on all channels in current nightly, so it should reach release in 79. If there are no issues we will keep the behaviour as default and remove the previous behaviour (what's currently on release). I guess MDN should really be updated for 79 since it's not going to be visible to the release users until then.

Regarding your comments, responding inline:

(In reply to Janet Swisher from comment #27)

:yzen : Is this description accurate? (Checking before I update the MDN docs)

If devtools.accessibility.auto-init.enabled is true, then:

  • Opening the Accessibility Inspector automatically enables accessibility features, with no other action required.

that's correct

  • A Show Accessibility Properties item is available in the context menu in the DOM and Style Inspector.

The difference in new functionality is that the context menu item will be there and enabled even if the accessibility panel has not been opened/started in advance. In terms of context menu items themselves, Show Accessibility Properties is DOM inspector, we will also have the Inspect Accessibility Properties menu item in main browser context menu and, again, will now show it even if the accessibility panel was not started/enabled in advance.

  • Clicking this item opens the Accessibility Inspector and displays properties of the element that was right-clicked in the DOM and Style Inspector, with no other action required to enable accessibility features.

Yes and the same for Inspect Accessibility Properties in the main browser context menu.

  • Switching from the Accessibility Inspector to another tool turns accessibility features off automatically, with no other action required.

This one is slightly different. Accessibility features will not be off automatically until the DevTools [toolbox] are closed. So in other words we turn them off but not when the user switches away from the accessibility tab, but when they close the toolbox (or all toolboxes if they have them open in more than one tab).

Flags: needinfo?(yzenevich) → needinfo?(jswisher)

Thanks for the clarifications!

Flags: needinfo?(jswisher)

Updated Accessing the Accessibility Inspector to include the information in comment #28.

Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: