GNOME Bugzilla – Bug 629884
magnifier defaults
Last modified: 2013-12-04 18:03:03 UTC
Our screen magnifier should default to fullscreen with "proportional" mouse tracking. We should also have global hotkeys for enabling/disabling, zooming in, and zooming out. The proportional mouse tracking can be seen in orca or in OS X.
So partly: gsettings set org.gnome.accessibility.magnifier screen-position \"full-screen\" gsettings set org.gnome.accessibility.magnifier mouse-tracking \"proportional\" And fix: diff --git a/js/ui/magnifier.js b/js/ui/magnifier.js index e572961..d68ab9c 100644 --- a/js/ui/magnifier.js +++ b/js/ui/magnifier.js @@ -15,8 +15,8 @@ const MagnifierDBus = imports.ui.magnifierDBus; const MouseTrackingMode = { NONE: 0, CENTERED: 1, - PUSH: 2, - PROPORTIONAL: 3 + PROPORTIONAL: 2, + PUSH: 3 }; We also have a bug in the proportional mode where it uses the full screen width and height to calculate the viewable area. This makes it hard to use the edges of the screen because there is insufficient periphery. There should be a pretty decent margin all the way around the edges of the screen. So for example, in order to activate the Activities hot corner you should only have to move the cursor to (say) 10% of the screen width from the edge of the monitor. We can experiment for the right value.
Created attachment 170447 [details] [review] magnifier: Change default values mccann: we should default to full screen with mouse position as "proportional"
Created attachment 170448 [details] [review] magnifier: Sync MouseTrackingMode values with the gsettings enum The tracking modes "push" and "proportional" are swapped - fix the code so that the setting behaves as expected.
Assigning to Joseph for review
Review of attachment 170447 [details] [review]: mccann: we should default to full screen with mouse position as "proportional" It should have defaulted to full screen for a while now. My bad. I have no opinion regarding the default mouse tracking mode. I asked our occupational therapist for her input. She advised that all things being equal, proportional cues location better than the other modes. However, I *think* Orca's preferences default to centred. I'll touch base with Joanie on this.
(In reply to comment #5) > I *think* > Orca's preferences default to centred. I'll touch base with Joanie on this. Joanie confirmed. But, she went on to suggest that the default should be "push".
I was actually asking for code review (though these patches are pretty simple). So, if you think the patches are good to commit, mark accepted-commit-now, if you think they need changes, describe what the changes are and mark needs-work.
Yeah we want proportional. I'll open a separate bug for fixing the proportional behavior.
Review of attachment 170448 [details] [review]: Orca uses centered=0, proportional=1, push=2, none=3. This would be a good time to synchronize with Orca.
Review of attachment 170447 [details] [review]: Code looks right.
Created attachment 170533 [details] [review] magnifier: Sync MouseTrackingMode values with the gsettings enum The tracking modes "push" and "proportional" are swapped - while at it, synchronize the values with Orca.
Isn't it a little strange to have: > NONE: 3 instead of > NONE: 0 Maybe we can't fix Orca though, but if we can, now is the time to fix this weird value...
Comment on attachment 170448 [details] [review] magnifier: Sync MouseTrackingMode values with the gsettings enum (In reply to comment #12) > Isn't it a little strange to have: > > NONE: 3 > instead of > > NONE: 0 > > Maybe we can't fix Orca though, but if we can, now is the time to fix this > weird value... Yeah, NONE:0 makes more sense to me. On the other hand, changing Orca likely is going to mess with users' settings, while the code change in gnome-shell is completely undisruptive (except maybe for some raised eyebrows).
Created attachment 170535 [details] [review] magnifier: Sync MouseTrackingMode values with the gsettings enum Adjust bounds check to use the changed values.
(In reply to comment #13) > Yeah, NONE:0 makes more sense to me. On the other hand, changing Orca likely is > going to mess with users' settings, while the code change in gnome-shell is > completely undisruptive (except maybe for some raised eyebrows). Am I correct that enums are stored as strings in GConf and GSettings, and that integer values are only internal representations? In this case, fixing the Shell and Orca wouldn't be a problem.
Created attachment 170553 [details] [review] magnifier: Sync MouseTrackingMode values with the gsettings enum Fix a syntax error.
Hey guys. FWIW, Orca generally does: DEFAULT_SETTING: 0 OTHER_SETTING1: 1 OTHER_SETTING2: 2 ... And in the case of mouse tracking, normally you want the mouse tracked *somehow* -- to not track the mouse at all would be "weird." Thus None is not default, and as a result, None is not 0. Having said that, no one should be accessing these settings by literal value. (And if they are, they get what they deserve. ;-) )Therefore, if it would make y'all happy to have Orca modified to fit your world view, go for it.
(In reply to comment #17) > Hey guys. FWIW, Orca generally does: > > DEFAULT_SETTING: 0 > OTHER_SETTING1: 1 > OTHER_SETTING2: 2 > ... > > And in the case of mouse tracking, normally you want the mouse tracked > *somehow* -- to not track the mouse at all would be "weird." Thus None is not > default, and as a result, None is not 0. Ah, makes sense then. Now the obvious question is whether it makes sense to change *both* the patch and Orca to account for the new default value ...
If you already have a consistent policy, don't worry about my remark - it would add inconsistency in Orca. Anyway, the value stored in GSettings is what matters, and it's a string...
What's the story here folks? Can we push this?
(In reply to comment #20) > What's the story here folks? Can we push this? Second patch is still waiting for review. Also, my question about whether it makes sense to change the enum values to match the "0 == default value" policy still applies ...
(In reply to comment #21) > (In reply to comment #20) > > What's the story here folks? Can we push this? > > Second patch is still waiting for review. Also, my question about whether it > makes sense to change the enum values to match the "0 == default value" policy > still applies ... Some thoughts. 1. Most of the uses of enumerated types in GNOME Shell use 0 for the "none" value, so switching to "0 = the default value" would introduce an inconsistency in style. 2. Joanie reports that Orca does not actually persist the numerical value, so, in that sense, there is no question about matching Orca's use of numeric constants for its preferences. We could simply ignore numeric values in the magnifier code -- that is, use GSettings' get/set_string() to acquire the 'nick names' of the enumeration. Admittedly, the ability to do range checks easily is lost, e.g.: 'if (setting >= NONE && setting <= MAX_ENUM)'. But, on the plus side, the whole issue of what to do with NONE, and how to be consistent with Orca simply goes away. The implication is to re-write parts of magnifier.js so as to use just the nick-names. I'll do that if others think that's a viable way to go.
*** Bug 633572 has been marked as a duplicate of this bug. ***
Created attachment 173567 [details] [review] magnifier: Sync MouseTrackingMode values with the gsettings enum (In reply to comment #22) > 1. Most of the uses of enumerated types in GNOME Shell use 0 for the "none" > value, so switching to "0 = the default value" would introduce an inconsistency > in style. OK, going back to 0 == NONE. > 2. Joanie reports that Orca does not actually persist the numerical value, so, > [...] > The implication is to re-write parts of magnifier.js so as to use just the > nick-names. I'll do that if others think that's a viable way to go. It probably is, but I think we should get the bug fix in nonetheless - having the settings behave as expected certainly wouldn't hurt while working on the code.
Review of attachment 173567 [details] [review]: Yes, this has been lingering a while. Let's commit this.
Attachment 170447 [details] pushed as 576376a - magnifier: Change default values Attachment 173567 [details] pushed as 0c2aa14 - magnifier: Sync MouseTrackingMode values with the gsettings enum I think the change of settings enums => nick-names should get its own bug, so closing this one ...