Closed
Bug 1081720
Opened 10 years ago
Closed 10 years ago
[AccessFu] Use prefs service to store current quicknav mode
Categories
(Core :: Disability Access APIs, defect)
Core
Disability Access APIs
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: eeejay, Assigned: eeejay)
References
Details
Attachments
(1 file)
14.70 KB,
patch
|
yzen
:
review+
fabrice
:
review+
|
Details | Diff | Splinter Review |
Storing it as a pref, and proxying it to content settings API will give us a consistent state across gaia and chrome to work with.
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8504426 -
Flags: review?(yzenevich)
Assignee | ||
Comment 2•10 years ago
|
||
Comment on attachment 8504426 [details] [diff] [review] Use prefs service to store quicknav state and proxy quicknav prefs to b2g settings. Adding Fabrice as reviewer for non-accessfu bits (namely settings.js)
Attachment #8504426 -
Flags: review?(fabrice)
Updated•10 years ago
|
Attachment #8504426 -
Flags: review?(fabrice) → review+
Comment 3•10 years ago
|
||
Comment on attachment 8504426 [details] [diff] [review] Use prefs service to store quicknav state and proxy quicknav prefs to b2g settings. Review of attachment 8504426 [details] [diff] [review]: ----------------------------------------------------------------- a couple of nits, otherwise looks good. ::: accessible/jsat/AccessFu.jsm @@ +20,5 @@ > const ACCESSFU_AUTO = 2; > > const SCREENREADER_SETTING = 'accessibility.screenreader'; > +const QUICKNAV_MODES_PREF = 'accessibility.accessfu.quicknav_modes'; > +const QUICKNAV_CURRENT_MODE_PREF = 'accessibility.accessfu.quicknav_current_mode'; Nit: Here and below ..._mode_index or just ..._index @@ +976,5 @@ > > next: function quickNavMode_next() { > + Services.prefs.setIntPref(QUICKNAV_CURRENT_MODE_PREF, > + this._currentIndex + 1 >= this.modes.length ? > + 0: this._currentIndex + 1); Nit: whitespace ::: accessible/tests/mochitest/jsat/jsatcommon.js @@ +44,5 @@ > // Ignore unexpected messages. > if (!(aMessage instanceof Components.interfaces.nsIConsoleMessage)) { > return; > } > + //info('comparing ' + aMessage.message + ' : ' + aWaitForMessage); Nit: whitespace @@ +157,5 @@ > Logger.logLevel = Logger.DEBUG; > }; > > + var prefs = [['accessibility.accessfu.notify_output', 1], > + ['dom.mozSettings.enabled', true]] Nit: ; @@ +158,5 @@ > }; > > + var prefs = [['accessibility.accessfu.notify_output', 1], > + ['dom.mozSettings.enabled', true]] > + Array.prototype.push.apply(prefs, aAdditionalPrefs); prefs.push.apply ::: accessible/tests/mochitest/jsat/test_quicknav_modes.html @@ +23,5 @@ > + return function() { > + is(AccessFu.Input.quickNavMode.current, aCurrentMode, > + 'initial current mode is correct'); > + AccessFu.Input.quickNavMode.next(); > + _expectMode(aNextMode, AccessFuTest.nextTest); I think we can move this before AccessFu.Input.quickNavMode.next(); This should let you get rid of lines 57 to 60 @@ +32,5 @@ > + return function() { > + is(AccessFu.Input.quickNavMode.current, aCurrentMode, > + 'initial current mode is correct'); > + AccessFu.Input.quickNavMode.previous(); > + _expectMode(aNextMode, AccessFuTest.nextTest); Same as above @@ +40,5 @@ > + function setMode(aModeIndex, aExpectedMode) { > + return function() { > + SpecialPowers.setIntPref( > + 'accessibility.accessfu.quicknav_current_mode', aModeIndex); > + _expectMode(aExpectedMode, AccessFuTest.nextTest); Same as above @@ +49,5 @@ > + SpecialPowers.setCharPref('accessibility.accessfu.quicknav_modes', > + 'Landmark,Button,Entry,Graphic'); > + // When the modes are reconfigured, the current mode should > + // be set to the first in the new list. > + _expectMode('Landmark', AccessFuTest.nextTest); Same as above. @@ +52,5 @@ > + // be set to the first in the new list. > + _expectMode('Landmark', AccessFuTest.nextTest); > + } > + > + function _expectMode(aExpectedMode, aCallback) { Only lines 61-63 should be necessary. @@ +68,5 @@ > + // Listen for initial 'EventManager.start' and disable AccessFu. > + function prefStop() { > + ok(AccessFu._enabled, "AccessFu was started via preference."); > + AccessFuTest.once_log("EventManager.stop", AccessFuTest.finish); > + SpecialPowers.setIntPref("accessibility.accessfu.activate", 0); Just like here ^ @@ +85,5 @@ > + AccessFuTest.addFunc(prefStop); > + AccessFuTest.waitForExplicitFinish(); > + AccessFuTest.runTests([ > + ['accessibility.accessfu.quicknav_modes', 'Link,Heading,FormElement'] > + ]); // Will call SimpleTest.finish(); Nit: whitespace
Attachment #8504426 -
Flags: review?(yzenevich) → review+
Assignee | ||
Comment 4•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/2fdd96ecd3e1
Comment 5•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2fdd96ecd3e1
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in
before you can comment on or make changes to this bug.
Description
•