1588895 - Global Default Zoom UI/UX

Status: RESOLVED FIXED

(Firefox :: Disability Access, enhancement, P2)

Description

[Asa Dotzler [:asa]]

Request Description
Global Default Zoom setting: where does this appear in preferences? what copy to use?

Deadline
ASAP

Priority Level
P1

Priority Level Description
This feature is in support of an Accessibility Q4 OKR

Supporting Information
https://trello.com/c/N75AtuDF/231-accessibility-default-page-zoom-option

There are various add-ons (e.g. NoSquint, Default FullZoom Level, ...) that allow the user to set a default zoom level for pages. However, I think we should consider having this built in to the Options (Preferences.)

People with poor eyesight, people with high-res LCD screens, or people with non-standard monitors may have to change the full-zoom setting for each and every domain that they visit.

This feature exists in Chrome.

https://docs.google.com/document/d/1CaxUCZI-Vy25YX_mkoiIaNyg91x_Vt_s_ki0BIYB8uk/edit#

Comment 1

[Eric Pang [:epang] UX]

Attached a spec to the user story section of this bug.
Here as well: https://mozilla.invisionapp.com/share/YEUH885FURH#/389043059_Default_Zoom
Please let me know if there are any questions or if any clarifications are needed.

Comment 2

[Morgan Reschenberg [:morgan]]

Some implementation questions given the UI/UX looks good :)

• We have a site-specific zoom pref (same-origin zooming). if this pref is false, right now we treat each page as having its own independent zoom state. if I add a global zoom preference, what should the following pref combos do?

  • siteSpecific && global
  • !siteSpecific && global
    my intuition:
  • siteSpecific && global --> use global as the base and allow adjustments made with meta + andmeta - to propagate through same-origin pages, but not change the global zoom
  • !siteSpecific && global --> use global as the base and do no propagation when adjustments are made. also: do not change global zoom.

• what's the difference between preferences declared in StaticPrefList.yaml and browser/app/profile/firefox.js?

• it looks like our zoom pref for site specific zoom is in the latter file and formatted as browser.zoom.siteSpecific; does browser.zoom.global feel like a fairly OK name? Should we do browser.zoom.default to keep in line with the UI/UX label?

• given the UI/UX and (hypothetical) creation of the pref, how do I "link" the two? I assume I can set the pref in tests using some variant of test-pref(...) but for more manual testing, I'm not sure how I'd go about turning the pref on and off. Unlike backplate, prefs in the JS file don't show up in about:config.

• what's the difference between zooming the entire page and zooming the text? (it looks like that's an option in our existing code and in the UI/UX mock above.

Comment 3

[James Teh [:Jamie]]

(In reply to Morgan Reschenberg [:morgan] from comment #2)

• We have a site-specific zoom pref (same-origin zooming). if this pref is false, right now we treat each page as having its own independent zoom state. if I add a global zoom preference, what should the following pref combos do?

I concur with your intuition here, but let's wait on Asa's response here too.

• what's the difference between preferences declared in StaticPrefList.yaml and browser/app/profile/firefox.js?

I'm not overly familiar with this, but I did a bit of digging. It looks like StaticPrefList.yaml is used for stuff that's primarily used by Gecko C++/Rust code, whereas profile/firefox.js is stuff specific to Firefox for desktop and queried from front-end. Note that front-end doesn't necessarily mean it's not a feature primarily implemented in Gecko. It could just mean that front-end is responsible for figuring out what and when to tell Gecko and then it makes a call which Gecko handles. I'm pretty sure this is what happens with full page zoom; see browser/base/content/browser-fullZoom.js. That is, it looks like the front-end code figures out what zoom factor to apply and then asks Gecko to apply it, rather than Gecko using the pref directly. In contrast, high contrast mode (see what I did there? 😜) seems to be handled entirely by Gecko, pref and all.

does browser.zoom.global feel like a fairly OK name? Should we do browser.zoom.default to keep in line with the UI/UX label?

Hmm. I prefer browser.zoom.global, but I also like being consistent with the UI where possible. So I'm grudgingly settling on browser.zoom.default.

• given the UI/UX and (hypothetical) creation of the pref, how do I "link" the two? I assume I can set the pref in tests using some variant of test-pref(...) but for more manual testing, I'm not sure how I'd go about turning the pref on and off. Unlike backplate, prefs in the JS file don't show up in about:config.

They... really should. When I implemented toolbar keyboard navigation (bug 1436086), the pref went in profile/firefox.js and it definitely shows up in about:config. Is it possible it didn't rebuild properly or something?

• what's the difference between zooming the entire page and zooming the text? (it looks like that's an option in our existing code and in the UI/UX mock above.

Um... I have no idea. On Windows at least, there's an option on the menu bar, View -> Zoom -> Zoom Text Only, unchecked by default. Note that the menu bar is hidden by default; press f10 to show it. I don't think this is exposed in the Firefox hamburger menu, which is weird... although it's also hard for me to tell because the zoom buttons in that menu are unlabelled 🤦‍♂️.

Comment 4

[Asif Youssuff]

Attachment: Screenshot from 2019-10-23 02-51-28.png

what's the difference between zooming the entire page and zooming the text? (it looks like that's an option in our existing code and in the UI/UX mock above.

See the screenshot attached. Basically, zooming the text keeps the page styling intact while only modifying the zoom level of the text.

Comment 5

[Asif Youssuff]

The Invision spec proposal that asks that the "two settings should stay in sync" (the proposed preference item and the menu item), feels very odd because generally, view menu changes do not modify "preferences" -- they are either transient or if permanent, are options only available in that area (like zoom on a specific site, as exists today). It feels odd to me to modify the preferences outside of the preferences UI.

In addition, see Bug 433489, which requests that the zoom text setting should be remembered per site, which as the ticket notes, makes a lot of sense, since many pages will break with the zoom text only setting enabled, while others might be fine.

It makes far more sense to me to roll up Bug 433489 as part of the new zoom features and to not have the menu item effect the global preference.

Comment 6

[:Gijs (out for now; he/him)]

(In reply to James Teh [:Jamie] from comment #3)

• what's the difference between preferences declared in StaticPrefList.yaml and browser/app/profile/firefox.js?

I'm not overly familiar with this, but I did a bit of digging. It looks like StaticPrefList.yaml is used for stuff that's primarily used by Gecko C++/Rust code, whereas profile/firefox.js is stuff specific to Firefox for desktop and queried from front-end. Note that front-end doesn't necessarily mean it's not a feature primarily implemented in Gecko. It could just mean that front-end is responsible for figuring out what and when to tell Gecko and then it makes a call which Gecko handles. I'm pretty sure this is what happens with full page zoom; see browser/base/content/browser-fullZoom.js. That is, it looks like the front-end code figures out what zoom factor to apply and then asks Gecko to apply it, rather than Gecko using the pref directly. In contrast, high contrast mode (see what I did there? 😜) seems to be handled entirely by Gecko, pref and all.

StaticPrefs have low-cost C++ accessors. Prefs generally (ie anywhere else) do not and require string arguments and hashtable lookups inside the prefs code, so StaticPrefs is better if you use the pref from C++. JS access always requires a hashtable lookup. This can be important if the code is on a hot path. To avoid this in JS, we use XPCOMUtils.defineLazyPreferenceGetter to define a variable that matches the pref value and gets kept up-to-date if the pref changes. See e.g. https://groups.google.com/d/msg/mozilla.dev.platform/kVZie-jL3sw/uizlUJvLAQAJ and https://groups.google.com/forum/#!msg/mozilla.dev.platform/n0F044PIKWw/gpFxx5YeAQAJ for some more context.

In terms of separation of desktop Firefox vs gecko, firefox.js is only applied in desktop Firefox, not in xpcshell tests or geckoview or other mozilla-central consumers. all.js and StaticPrefList.yaml is applied everywhere. modules/libpref/init/all.js is the equivalent of firefox.js that is usable in all of gecko. There are also browser. (ie Firefox) prefs in StaticPrefs, ie https://searchfox.org/mozilla-central/rev/9df2ccc256fe54c314b34e52920471a0d04b283e/modules/libpref/moz.build#32 .

(In reply to James Teh [:Jamie] from comment #3)

I don't think this is exposed in the Firefox hamburger menu, which is weird...

I think full page zoom came after text zoom, and is generally considered "better" - pages often don't cope well with "only" changing text sizes.

although it's also hard for me to tell because the zoom buttons in that menu are unlabelled 🤦‍♂️.

Did you file a bug for this? We should fix it.

Comment 7

[James Teh [:Jamie]]

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

although it's also hard for me to tell because the zoom buttons in that menu are unlabelled 🤦‍♂️.

Did you file a bug for this? We should fix it.

I didn't yet, but apparently Morgan is working on a patch for this, so she'll file soon.

Comment 8

[Morgan Reschenberg [:morgan]]

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

Did you file a bug for this? We should fix it.

Bug and patch here! https://bugzilla.mozilla.org/show_bug.cgi?id=1592719
Tagged you for review because Jamie mentioned that might be the right thing to do. Looks like I might also need a l10n reviewer, but this is an older (non-fluent) file. Any ideas on who I should tag for that?

Comment 9

[:Gijs (out for now; he/him)]

(In reply to Morgan Reschenberg [:morgan] from comment #8)

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

Did you file a bug for this? We should fix it.

Bug and patch here! https://bugzilla.mozilla.org/show_bug.cgi?id=1592719
Tagged you for review because Jamie mentioned that might be the right thing to do. Looks like I might also need a l10n reviewer, but this is an older (non-fluent) file. Any ideas on who I should tag for that?

I'm in the FTL reviewer group, so I'll just review all of it. :-)

Comment 10

[James Teh [:Jamie]]

We landed the feature, so i think we can close this.