Open Bug 539417 Opened 15 years ago Updated 2 years ago

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

Categories

(Toolkit :: General, defect)

defect

Tracking

()

People

(Reporter: mikey, Unassigned)

References

Details

(Whiteboard: [has draft patch])

Attachments

(1 file)

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.
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.
Version: unspecified → Trunk
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Other → All
Hardware: x86_64 → All
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.
Product: Core → Toolkit
QA Contact: general → general
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.
Summary: Reset Zoom (Ctrl+0) does nothing if zoom.MinPercent > 100 → Reset Zoom (Ctrl+0) does nothing if zoom.MinPercent > 100, throws exception
Attached patch Draft for partial solution (deleted) — Splinter Review
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.
Whiteboard: [has draft patch]
Severity: minor → S4
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: