539417 - Reset Zoom (Ctrl+0) does nothing if zoom.MinPercent > 100, throws exception

Status: NEW

(Toolkit :: General, defect)

Description

[Mikey Cooper]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1.7) Gecko/20091221 Firefox/3.5.7 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1.7) Gecko/20091221 Firefox/3.5.7 (.NET CLR 3.5.30729)

I have set my zoom.minPercent to 120 so that all web pages are zoomed a little by default.

When I zoom in further on some web pages and then attempt to reset my zoom using View -> Zoom -> Reset or by hitting Ctrl+0, nothing happens.

Reproducible: Always

Steps to Reproduce:
1. Set zoom.MinPercent to 120 in about:config.
2. Restart Firefox.
3. Go to a mozilla.org and zoom in.
4. Hit Ctrl+0 to reset the zoom.
Actual Results:  
The zoom level remains unchanged.

Expected Results:  
I expected it to attempt to set the zoom back to 120% since 100% is no longer in range and 120% is the closest value.

Comment 1

[Ria Klaassen (not reading all bugmail)]

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1.8pre) Gecko/20100112 Shiretoko/3.5.8pre

Confirmed that the browser doesn't react to ctrl+0

Also with the latest trunk build.

Comment 2

[rsx11m]

ZoomManager.reset in viewZoomOverlay.js doesn't obey ZoomManager.min or .max, which may be the cause for this (or a separate but closely related bug).

Also, ZoomManager is toolkit, moving there.

Comment 3

[rsx11m]

I'm getting an exception when using Ctrl+0 with zoom.minPercent=101, thus my guess is that ZoomManager.reset sets this.zoom = 1, then setZoomForBrowser is called and checks it against this.MIN and .MAX, raising NS_ERROR_INVALID_ARG.

It may be more prudent to silently correct aVal in setZoomForBrowser to be within the MIN/MAX range rather than throwing an exception. A quick search shows that there are other calls for setZoomForBrowser with a hard-wired aVal=1, thus likely causing the same issue.

Comment 4

[rsx11m]

Attachment: Draft for partial solution

This is derived from the bug 672146 solution for SeaMonkey. The modifications to browser-fullZoom.js are untested and correspond to hunks #2 and #3 in the patch for that bug; they catch the 100% cases for which currently exceptions are thrown here. This also ensures that pages come up with a valid zoom level upon opening of a new tab or browser window.

The additions to reset() in viewZoomOverlay.js address the specific issue of Ctrl+0 not working correctly by returning this.MIN or this.MAX if 100% is outside the allowed range. This part was successfully tested in SeaMonkey with hunk #1 of the original patch omitted.

Not covered is an issue with new tabs or windows which are not subject to site-specific zoom levels. Those are still opening at 100% even if not allowed by the zoom.{min,max}Percent settings. I was unable to locate where this is set, but no exception is thrown in these cases, so it's apparently an initialization issue rather than caused by a call to setZoomForBrowser().

Anybody feel free to take it from here and finalize the patch with missing cases and tests as required.