Closed
Bug 813946
Opened 12 years ago
Closed 12 years ago
Zooming is enabled on ebay.com (mobile page)
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox20 verified)
VERIFIED
FIXED
Firefox 20
Tracking | Status | |
---|---|---|
firefox20 | --- | verified |
People
(Reporter: xti, Assigned: groodt)
Details
(Whiteboard: [mentor=kats][lang=js])
Attachments
(1 file, 1 obsolete file)
1.83 KB,
patch
|
kats
:
review+
|
Details | Diff | Splinter Review |
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.
Comment 1•12 years ago
|
||
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]
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.
Updated•12 years ago
|
Attachment #687293 -
Attachment is patch: true
Comment 4•12 years ago
|
||
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+
Updated•12 years ago
|
Assignee: nobody → groodt
Attachment #687293 -
Attachment is obsolete: true
Comment 6•12 years ago
|
||
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+
Updated•12 years ago
|
Keywords: checkin-needed
Comment 8•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/6ce882e41eb1
Keywords: checkin-needed
Comment 9•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/6ce882e41eb1
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 20
Reporter | ||
Comment 10•12 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
Comment 11•12 years ago
|
||
Nice job, groodt! \o/
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•