1277719 - Maximum image zoom level seems more restricted than before

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Toolbar, defect)

Description

[Jan Henning [:JanH]]

Viewing an image, I can no longer zoom in as much as I used to. This currently affects Nightly and Aurora, so can I blame APZ?

Comment 1

[Jan Henning [:JanH]]

Attachment: Previous maximum zoom level

Comment 2

[Jan Henning [:JanH]]

Attachment: Current maximum zoom level

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Yeah, probably APZ-related. There's a MAX_ZOOM const in AsyncPanZoomController.cpp which is probably what's clamping it here. We could try bumping that up and see if it helps.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Yeah, cranking that MAX_ZOOM up to 16.0 allows zooming in to the same level as the Java PZC. I think the kViewportMaxScale at [1] becomes the next limiting factor, which is fine. So if we put the MAX_ZOOM at 10.0 it should fix this bug without introducing unnecessary zoom power. And since Fennec is currently the only platform that supports pinch-zooming in APZ we might as well just make the change.

[1] http://mxr.mozilla.org/mozilla-central/source/dom/base/nsViewportInfo.h?rev=37d64688bf9d#16

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Actually we do need it to go all the way to 16.0, the kViewportMaxScale is in different units.

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1277719 - Remove unnecessary duplicated zoom bounds from APZC and make nsViewportInfo have the source of truth for those values.

Review commit: https://reviewboard.mozilla.org/r/57666/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/57666/

Comment 7

[Randall Barker [:rbarker]]

Comment on attachment 8759788 [details]
Bug 1277719 - Remove unnecessary duplicated zoom bounds from APZC and make nsViewportInfo have the source of truth for those values.

https://reviewboard.mozilla.org/r/57666/#review54504

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6e84b5b95e3a
Remove unnecessary duplicated zoom bounds from APZC and make nsViewportInfo have the source of truth for those values. r=rbarker

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6e84b5b95e3a

Comment 10

[Sorina Florean [:sflorean]]

Verified as fixed in build 49.0a2 (2016-06-15);
Device: 
- Asus ZenPad 8 (Android 5.0.2);
- Nexus 5 (Android 6.0.1).

Comment 11

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8759788 [details]
Bug 1277719 - Remove unnecessary duplicated zoom bounds from APZC and make nsViewportInfo have the source of truth for those values.

Approval Request Comment
[Feature/regressing bug #]: APZ
[User impact if declined]: User is unable to zoom in on imageas as much as they could before
[Describe test coverage new/current, TreeHerder]: verified manually
[Risks and why]: fairly low risk, small change
[String/UUID change made/needed]: none

Comment 12

[Gerry Chang [:gchang]]

Comment on attachment 8759788 [details]
Bug 1277719 - Remove unnecessary duplicated zoom bounds from APZC and make nsViewportInfo have the source of truth for those values.

This patch fixes the regression. Take it in 48 beta 3.

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/edb9d56123ee

Comment 14

[u421692]

Verified as fixed on Firefox 48 Beta 3