1251874 - Allow modification of minimum/maximum zoom levels in Content preference pane

Status: RESOLVED FIXED

(SeaMonkey :: Preferences, enhancement)

Description

[rsx11m]

+++ This bug was initially created as a clone of Bug #851898 +++

In parity with Firefox, the zoom levels are currently restricted to 30-300% by default. Bug 851898 intends to make the full range available through the Ctrl/+ and Ctrl/- keyboard shortcuts, and to preemptively extend the range of zoom levels to 20-800%. The purpose of this bug is to make the underlying zoom.{min,max}Percent preferences available in the Preferences settings.

Appearance > Content appears to be a good fit, given that zoom affects all content and not just web pages. We also have a couple of preference there already relating to zoom:

[ ] Zoom only text instead of full pages
[ ] Remember zoom levels on a per-site basis
[ ] Resize large images to fit in the browser window (*)

(*) this may actually apply to attachments in mail/news messages as well, in which case I'd just remove "browser" from the label.

It seems best to create a new groupbox in this prefpane for the zoom options, then adding the minimum/maximum boxes. Options for the latter are:

(A) Simple number/text boxes with dials, allowing to freely enter the levels (where min>100% is actually a use case for enforcing a larger-than-100% zoom by default for accessibility reasons, given that NoSquint is no longer supported for SeaMonkey by the author of that add-on).

(B) Listboxes pre-filled with the levels of toolkit.zoomManager.zoomValues to ensure that the preferences always match a valid level and show up in the menus.

For the special case that min>max after a change, either the change has to be disallowed or the other value being adjusted to match the change.

Comment 1

[rsx11m]

Attachment: WIP Screenshot

This is what I have in mind for the new Content prefpane layout. The "Website icons" groupbox remains untouched, then we have the new "Zoom options" groupbox. For the remaining options, I made a new "General options" groupbox to keep things reasonably consistent. No labels or accesskeys were changed.

The zoom.minPercent and zoom.maxPercent prefs are presented as a textbox with increment="10" spinbuttons. I kept them in a single line for reasons of space, thus avoiding a two-row grid for "Minimum zoom level" and "Maximum zoom level" aligned settings. While I'm aware that such constructs are more complicated for localization, the "from"/"to" is hopefully simple enough to be represented in all locales.

I've looked into menuboxes as alternative, dynamically filled from the zoomLevels preference. That should work, but doesn't allow entering values other than those defined in the pref. Making the menubox editable would resolve that, but in contrast to a textbox, doesn't allow restriction to numerical values and ranges. Thus, I prefer the textbox. It would be neat if the spinbuttons could increase or decrease the current value to the next higher or lower preset zoom level, but this doesn't look trivial to me. I could add an event listener to the spinbuttons element, thus capturing that event, but I'm not sure where I'd go from there.

Comment 2

[rsx11m]

> [ ] Resize large images to fit in the browser window (*)
> 
> (*) this may actually apply to attachments in mail/news messages as well, in which case I'd just
> remove "browser" from the label.

It doesn't, thus no change here.

Comment 3

[rsx11m]

Meh, old build - bug 1218343 just added a new checkbox to this pane last week... :-\
So, either we keep a 2-box "Scrolling" group or that option goes into "General options" as well.

Comment 4

[rsx11m]

Attachment: Updated screenshot

I've got rid of the "General options" groupbox again to gain sufficient space for the new "Zoom options" without touching the recently introduced "Scrolling" groupbox. This /just/ works with respect to vertical space.

I'll post the patch once I'm done with the Help updates.

Comment 5

[rsx11m]

Attachment: Possible patch

This implements attachment 8730057 [details] but I don't like the behavior when typing in a number directly. Given the min="..." attributes, the number is forced to obey it immediately. Thus, trying to type in "120" it will force it to "50" when typing in the "1" for the "to" value, making it impossible to easily get there. The next patch tries to resolve this issue.

Comment 6

[rsx11m]

Attachment: Proposed patch

This introduces a new function DisableMinCheck(this); which suspends the "min" attribute with the oninput event when the user starts to type in a number. With the onchange event (i.e., switching focus), the "min" attribute is enforced again and may adjust the entered value if it's below the limit.

Using the spinbuttons or cursor keys, the behavior is equivalent to attachment 8730206 [details] [diff] [review].

Note that I'm allowing the minimum to go up to 200% and the maximum to 50%, thus accounting for possible accessibility use cases.

Drive-by fixes:
 - editorial change to the "Aggressively..." label to avoid wrapping in Windows Classic
 - introduced the "Scrolling" heading in Help which was missed in bug 1218343
 - dropped description of removed "Display website icons in bookmarks menu and toolbar"

Comment 7

[rsx11m]

Attachment: Proposed patch (v2)

Two more changes, Help content only:

 - Since I'm linking the Mouse Wheel, I've linked now Keyboard Shortcuts as well;
 - while there, added "Ctrl+0 (zero)" given that "+" and "-" are explained too
   (and "O" vs. "0" is frequently ambiguous in many fonts).

Comment 8

[Philip Chee]

Comment on attachment 8730390 [details] [diff] [review]
Proposed patch (v2)

No complaints. ui-r=me

Comment 9

[Ian Neal]

Comment on attachment 8730390 [details] [diff] [review]
Proposed patch (v2)

Does Firefox have to restart for the new zoom min and max to take affect?

Comment 10

[Philip Chee]

(In reply to Ian Neal from comment #9)
> Comment on attachment 8730390 [details] [diff] [review]
> Proposed patch (v2)
> 
> Does Firefox have to restart for the new zoom min and max to take affect?
AFAICT: Firefox needs a restart. It would be trivial to add a pref observer to our code but is out of the scope of this bug.

Comment 11

[rsx11m]

Yes, I've tested it Firefox behaves the same way (on Linux at least).

Comment 12

[rsx11m]

Filed bug 1260315 on comment #10.

Comment 13

[rsx11m]

Pushed to trunk, http://hg.mozilla.org/comm-central/rev/b654ace3f360