Closed
Bug 705983
Opened 8 years ago
Closed 8 years ago
Firefox a11y is not enabled on GNOME if config.use_system_prefs.accessibility is false (default)
Categories
(Core :: Disability Access APIs, defect, major)
Tracking
()
RESOLVED
FIXED
mozilla11
People
(Reporter: ginnchen+exoracle, Assigned: tbsaunde)
References
Details
Attachments
(2 files)
4.99 KB,
patch
|
ginnchen+exoracle
:
review+
roc
:
review+
christian
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
1.01 KB,
patch
|
ginnchen+exoracle
:
review+
|
Details | Diff | Splinter Review |
After landing Bug 451161, Firefox is no longer a11y enabled by default on GNOME even if GNOME a11y is enabled. We should have the behavior as we did before. Also this pref only used in ATK code, I think we should add #ifdef.
Comment 1•8 years ago
|
||
Strange. I was not meant to change the behavior by the patch. Had Firefox enabled a11y even if both config.use_system_prefs and config.use_system_prefs.accessibility is false (default)?
Comment 2•8 years ago
|
||
And I doubt the usefulness of the pref because nobody cared about the brokenness for a long time.
(In reply to Masatoshi Kimura [:emk] from comment #1) > Strange. I was not meant to change the behavior by the patch. > Had Firefox enabled a11y even if both config.use_system_prefs and > config.use_system_prefs.accessibility is false (default)? (Have GNOME a11y enable,) without bug 451161, yes with bug 451161, no The pref is just a mapping to GNOME conf /desktop/gnome/interface/accessibility Since you have already killed extensions/pref/system-pref. I think we can just remove it.
Summary: config.use_system_prefs.accessibility should be true by default → Firefox a11y is not enabled on GNOME if config.use_system_prefs.accessibility is false (default)
Comment 4•8 years ago
|
||
(In reply to Ginn Chen from comment #3) > (In reply to Masatoshi Kimura [:emk] from comment #1) > > Strange. I was not meant to change the behavior by the patch. > > Had Firefox enabled a11y even if both config.use_system_prefs and > > config.use_system_prefs.accessibility is false (default)? > (Have GNOME a11y enable,) > without bug 451161, yes > with bug 451161, no So, the prefs had not worked as expected. If config.use_system_prefs is false (default), config.use_system_prefs.accessibility is not supposed map /desktop/gnome/interface/accessibility. That is, a11y is supposed to be always disabled even before the patch. > The pref is just a mapping to GNOME conf > /desktop/gnome/interface/accessibility > Since you have already killed extensions/pref/system-pref. I updated a patch so that config.use_system_prefs* didn't rely on the system-pref. However, config.use_system_prefs.accessibility hadn't been respected because of system-pref's bug. > I think we can just remove it. I agree the conclusion because nobody hadn't noticed that the pref had not worked.
Assignee | ||
Comment 5•8 years ago
|
||
having firefox specific prefs may be a nice to have at some point but for know always using system prefs seems reasonable.
Attachment #578428 -
Flags: review?(surkov.alexander)
Attachment #578428 -
Flags: review?(roc)
Assignee | ||
Comment 6•8 years ago
|
||
Attachment #578474 -
Flags: review?(surkov.alexander)
Attachment #578428 -
Flags: review?(roc) → review+
Comment 7•8 years ago
|
||
Comment on attachment 578428 [details] [diff] [review] bug 705983 always check gconf for accessibility state when GNOME_ACCESSIBILITY not set I prefer if Ginn takes a review
Attachment #578428 -
Flags: review?(surkov.alexander)
Attachment #578428 -
Flags: review?(ginn.chen)
Comment 8•8 years ago
|
||
Comment on attachment 578474 [details] [diff] [review] bug 705983 (part 2) - remove the config.use_system_prefs.accessibility pref Ginn, I hope that's ok :)
Attachment #578474 -
Flags: review?(surkov.alexander) → review?(ginn.chen)
Comment 9•8 years ago
|
||
Comment on attachment 578428 [details] [diff] [review] bug 705983 always check gconf for accessibility state when GNOME_ACCESSIBILITY not set Don't forget removing unused values from modules/libpref/src/init/all.js.
Comment 10•8 years ago
|
||
Comment on attachment 578474 [details] [diff] [review] bug 705983 (part 2) - remove the config.use_system_prefs.accessibility pref "config.use_system_prefs" is no longer used at all.
Assignee | ||
Comment 11•8 years ago
|
||
(In reply to Masatoshi Kimura [:emk] from comment #10) > Comment on attachment 578474 [details] [diff] [review] [diff] [details] [review] > bug 705983 (part 2) - remove the config.use_system_prefs.accessibility pref > > "config.use_system_prefs" is no longer used at all. ok, that wasn't completely clear to me, thanks. Ginn, do you want a second patch 2, or should I just remove config.use_system_prefs pref before landing? (In reply to alexander :surkov from comment #8) > Comment on attachment 578474 [details] [diff] [review] [diff] [details] [review] > bug 705983 (part 2) - remove the config.use_system_prefs.accessibility pref > > Ginn, I hope that's ok :) I think comment 3 was suggesting this approach fwiw
Reporter | ||
Comment 12•8 years ago
|
||
Comment on attachment 578474 [details] [diff] [review] bug 705983 (part 2) - remove the config.use_system_prefs.accessibility pref Please remove config.use_system_prefs, too. You can do it when you commit the code.
Attachment #578474 -
Flags: review?(ginn.chen) → review+
Reporter | ||
Comment 13•8 years ago
|
||
Comment on attachment 578428 [details] [diff] [review] bug 705983 always check gconf for accessibility state when GNOME_ACCESSIBILITY not set thanks for the patch!
Attachment #578428 -
Flags: review?(ginn.chen) → review+
Assignee | ||
Comment 14•8 years ago
|
||
inbound https://hg.mozilla.org/integration/mozilla-inbound/rev/549b2d9592ce
Comment 15•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/549b2d9592ce
Assignee: nobody → trev.saunders
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
Comment 16•8 years ago
|
||
Comment on attachment 578428 [details] [diff] [review] bug 705983 always check gconf for accessibility state when GNOME_ACCESSIBILITY not set [Approval Request Comment] Regression caused by (bug #451161): User impact if declined: Accessibility will be always disabled on GNOME by default. Testing completed (on m-c and m-a): Risk to taking this patch (and alternatives if risky): I believe the risk is very small because the pref was broken from the start. This patch also needs an approval if the bug 451161 is approved.
Attachment #578428 -
Flags: approval-mozilla-beta?
Comment 17•8 years ago
|
||
Comment on attachment 578428 [details] [diff] [review] bug 705983 always check gconf for accessibility state when GNOME_ACCESSIBILITY not set [Triage Comment] Approving this for beta as we approved bug 451161 and this is a regression from it.
Attachment #578428 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in
before you can comment on or make changes to this bug.
Description
•