The default bug view has changed. See this FAQ.

Zooming is enabled on ebay.com (mobile page)

VERIFIED FIXED in Firefox 20

Status

()

Firefox for Android
General
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: xti, Assigned: groodt)

Tracking

20 Branch
Firefox 20
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox20 verified)

Details

(Whiteboard: [mentor=kats][lang=js])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

4 years ago
Firefox 20.0a1 (2012-11-20)
Device: Galaxy S2
OS: Android 4.0.3

Steps to reproduce:
1. Go to ebay.com
2. Pinch-to-zoom in/out

Expected result:
Zooming is disabled for mobile pages

Actual result:
Zooming is enabled for ebay.com

Note:
This issue cannot be reproduced on the stock browser.
They have a meta-viewport tag:

width=device-width, initial-scale=1.0, maximum-scale=1.0, minimum-scale=1.0

this doesn't explicitly set user-scalable=no, so we allow pinching. However they set a min and max scale, so we don't allow actually changing the zoom. This is why our behaviour is a little weird when pinching the page - it'll pinch as long as you have your fingers on but will normalize back to 1.0 when you let go.

I guess we could fix this in browser.js (the getViewportMetadata code) by automatically setting allowZoom = false when minScale and maxScale are both defined and equal to each other.
Whiteboard: [mentor=kats][lang=js]
(Assignee)

Comment 2

4 years ago
Created attachment 687293 [details] [diff] [review]
Patch to disable zoom when minScale and maxScale are equal.

Intention is to only allow zoom if minScale != maxScale.
(Assignee)

Comment 3

4 years ago
Tested on my Nexus 7 on the ebay mobile site and it does indeed disable the odd zooming behavior. 

Other sites seem to work correctly.
Attachment #687293 - Attachment is patch: true
Comment on attachment 687293 [details] [diff] [review]
Patch to disable zoom when minScale and maxScale are equal.

Review of attachment 687293 [details] [diff] [review]:
-----------------------------------------------------------------

This looks fine to me. It might be nice to expand that Webkit comment to also mention what we're doing with the new conditional clause. Bonus points if you also point out that NaN != NaN in javascript so if both minScale and maxScale are undefined that clause has no effect.

Please upload an updated patch ready for submission (i.e. includes a commit message with bug number and short description) following the guidelines at https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F
Attachment #687293 - Flags: feedback+
Assignee: nobody → groodt
(Assignee)

Comment 5

4 years ago
Created attachment 687533 [details] [diff] [review]
Patch for Bug 813946
Attachment #687293 - Attachment is obsolete: true
Comment on attachment 687533 [details] [diff] [review]
Patch for Bug 813946

Review of attachment 687533 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM. Usually when you have a patch you want to be reviewed you should explicitly request review by using the attachment flags (set the review flag to '?' and enter the name of the person you would like to review it).
Attachment #687533 - Flags: review+
Keywords: checkin-needed
(Assignee)

Comment 7

4 years ago
Thanks. I'll do that in future.
https://hg.mozilla.org/integration/mozilla-inbound/rev/6ce882e41eb1
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/6ce882e41eb1
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 20
(Reporter)

Comment 10

4 years ago
I cannot zoom the page anymore on the latest Nightly. Closing bug as verified fixed on:

Firefox 20.0a1 (2012-12-06)
Device: Galaxy Tab2 7"
OS: Android 4.0.3
Status: RESOLVED → VERIFIED
status-firefox20: --- → verified
Nice job, groodt! \o/
You need to log in before you can comment on or make changes to this bug.