Closed Bug 555196 Opened 10 years ago Closed 10 years ago

Regression: Zoom in/out commands break when zoom level is 1.0

Categories

(Firefox for Android Graveyard :: Panning/Zooming, defect, major)

x86
Linux
defect
Not set
major

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: mbrubeck, Assigned: mbrubeck)

Details

Attachments

(1 file)

For bug 454456, we are now forcing the zoom level to 1.0 whenever it is between 0.8 and 1.2.

This caused a regression: When the zoom level is 1.0, zoom in/out are broken.  The browser tries to zoom to 0.9 or 1.1, but is forced back to 1.0.
Attached patch patchSplinter Review
Here's a quick fix; we still adjust the page zoom near 1.0 while zooming in or out, but we're less aggressive so that we never adjust it by a whole zoom increment or more.
Assignee: nobody → mbrubeck
Status: NEW → ASSIGNED
Attachment #435159 - Flags: review?(mark.finkle)
Comment on attachment 435159 [details] [diff] [review]
patch

>diff -r e1136b98004d chrome/content/browser.js
>   zoom: function zoom(aDirection) {
>     let bv = this._browserView;
>-    let zoomLevel = bv.getZoomLevel() + (aDirection > 0 ? -0.1 : 0.1);
>-    let adjusted = BrowserView.Util.adjustZoomLevel(zoomLevel);
>+    let zoomLevel = bv.getZoomLevel() + (aDirection > 0 ? -1 : 1) * kBrowserViewZoomLevelIncrement; let adjusted = BrowserView.Util.adjustZoomLevel(zoomLevel, kBrowserViewZoomLevelIncrement/2);

Did you want to divide kBrowserViewZoomLevelIncrement by two when adjusting?

I'll fix the code layout when I land it
Attachment #435159 - Flags: review?(mark.finkle) → review+
Yes, dividing by two was a simple way to protect against weird floating-point problems like (1.0 - 0.9) < 0.1.

Thanks for fixing my layout goof.
Keywords: checkin-needed
pushed:
http://hg.mozilla.org/mobile-browser/rev/a6b7a96ea6e5
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Is there a test case for the regression found in comment #0 ?
I used the firefox support page to test this patch. verified FIXED on builds:

Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.2.3pre) Gecko/20100329 Namoroka/3.6.3pre Fennec/1.1a2pre

and

Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.3a4pre) Gecko/20100329 Namoroka/3.7a4pre Fennec/1.1a2pre
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.