Closed
Bug 750528
Opened 9 years ago
Closed 9 years ago
[AccessFu] Add preference for enabling/disabling AccessFu
Categories
(Core :: Disability Access APIs, defect)
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: eeejay, Assigned: eeejay)
References
Details
Attachments
(1 file, 2 obsolete files)
I am thinking accessibility.accessfu (int) 0: Disabled 1: Enabled 2: Auto Auto is the default and it will sniff platform a11y and enabled or disable AccessFu. On platforms that either don't have platform sniffing enabled in AccessFu (Mac, Windows, Linux), or platforms that don't have "platform a11y" outside of Firefox (B2G), auto is synonymous with disabled.
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → eitan
OS: Linux → Android
Hardware: x86_64 → ARM
Target Milestone: --- → mozilla15
Assignee | ||
Comment 1•9 years ago
|
||
This was a bit too easy. But it seems to work.
Attachment #620900 -
Flags: review?(dbolter)
Comment 2•9 years ago
|
||
Comment on attachment 620900 [details] [diff] [review] Use accessibility.accessfu to enable or disable accessfu. Review of attachment 620900 [details] [diff] [review]: ----------------------------------------------------------------- r=me Please consider my thoughts below: BTW Do you run "gjslint --nojsdoc my_file.js" ? (Just curious) ::: accessible/src/jsat/AccessFu.jsm @@ +17,5 @@ > Cu.import('resource://gre/modules/accessibility/VirtualCursorController.jsm'); > > +const ACCESSFU_DISABLED = 0; > +const ACCESSFU_ENABLED = 1; > +const ACCESSFU_AUTO = 2; Note, to keep my brain happier I'd consider these prefs rather than modes throughout (as in ACCESSFU_DISABLE, ACCESSFU_ENABLE, ACCESSFU_AUTO throughout). @@ +41,5 @@ > + this.prefsBranch = Cc["@mozilla.org/preferences-service;1"] > + .getService(Ci.nsIPrefService).getBranch("accessibility."); > + this.prefsBranch.addObserver('accessfu', this, false); > + > + let accessfuMode = ACCESSFU_DISABLED; let accessPref = ACCESSFU_DISABLE; ? @@ +49,3 @@ > } > > + if (this.checkAccessibility(accessfuMode)) Optional: since it returns a boolean I'd prefer an "is" name like "isWanted" or "amINeeded" (note: in 3 places). @@ +91,5 @@ > this.chromeWin.removeEventListener('scroll', this); > this.chromeWin.removeEventListener('TabOpen', this); > }, > > + checkAccessibility: function(aMode) { s/aMode/aPref ?
Attachment #620900 -
Flags: review?(dbolter) → review+
Assignee | ||
Comment 3•9 years ago
|
||
(In reply to David Bolter [:davidb] from comment #2) > Comment on attachment 620900 [details] [diff] [review] > @@ +49,3 @@ > > } > > > > + if (this.checkAccessibility(accessfuMode)) > > Optional: since it returns a boolean I'd prefer an "is" name like "isWanted" > or "amINeeded" (note: in 3 places). > thanks! I changed it to isEnabled. All the rest of the names are adjusted too.
Assignee | ||
Comment 4•9 years ago
|
||
(In reply to David Bolter [:davidb] from comment #2) > BTW Do you run "gjslint --nojsdoc my_file.js" ? (Just curious) > yes, although i think i missed it in the text event patch from the other day. I run: gjslint --additional_extensions=jsm --nojsdoc *.jsm
Assignee | ||
Comment 5•9 years ago
|
||
Attachment #620900 -
Attachment is obsolete: true
Assignee | ||
Comment 6•9 years ago
|
||
Attachment #620930 -
Attachment is obsolete: true
Assignee | ||
Comment 7•9 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/7972b92116b0
Comment 8•9 years ago
|
||
Comment on attachment 620931 [details] [diff] [review] Bug 750528 - Use accessibility.accessfu to enable or disable accessfu. r=davidb Review of attachment 620931 [details] [diff] [review]: ----------------------------------------------------------------- ::: accessible/src/jsat/AccessFu.jsm @@ +49,3 @@ > } > > + if (this.amINeeded(accessPref)) at some point David should try to write lyrics instead code ;) alternatively named functions aren't always great for code reading. @@ +52,1 @@ > this.enable(); I'd keep enable a part of amINeeded since it seems nobody need to turn it on/off explicitly.
Comment 9•9 years ago
|
||
Eitan if you happen to follow Alexander and move the enable call inside the boolean function that checks preferences please rename it so that the side effect (yuck) is obvious.
Comment 10•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7972b92116b0
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 11•9 years ago
|
||
(In reply to Ed Morley [:edmorley] from comment #10) > https://hg.mozilla.org/mozilla-central/rev/7972b92116b0 Wrong bug!
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 12•9 years ago
|
||
Oops. my bad
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•