1197824 - Update Fennec code to use ZoomConstraintsClient instead of the big pile of browser.js code

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Toolbar, defect)

Comment 1

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

Attachment: Part 1 - Stop propagating the defaultZoom value unnecessarily

Comment 2

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

Attachment: Part 2 - Constrain the min/max zoom if zooming is not allowed

Comment 3

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

Attachment: Part 3 - Some cleanup in GetViewportInfo

Comment 4

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

Attachment: Part 4 - Allow zooming in desktop mode

Comment 5

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

Attachment: Part 5 - Support the force-user-scalable pref in ZCC codepaths

Comment 6

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

Attachment: Part 6 - Send a update-zoom-constraints message to Fennec

Comment 7

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

Attachment: Part 7 - Update browser.js to use the new message and throw out unneeded code

Comment 8

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8655068 [details] [diff] [review]
Part 7 - Update browser.js to use the new message and throw out unneeded code

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

This is lovely.

Comment 9

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

I did a try push at https://treeherder.mozilla.org/#/jobs?repo=try&revision=a0ab9d0715a1&group_state=expanded which showed R6 failures on Android. I'm bisecting the queue to figure out where it's coming from (seems to be in the first half somewhere, possibly from the patches for bug 1200303 or bug 1200402).

Comment 10

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

Updated try push at [1] is green. There was an error in part 1 which resulted in the JavaPanZoomController using maxZoom = 4.0 instead of maxZoom = 0. Correcting that (using the incremental change at [2] fixed the problem.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=bb1275e8e541
[2] https://hg.mozilla.org/try/rev/806ca7d0f012

Comment 11

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

Attachment: Part 1 - Stop propagating the defaultZoom value unnecessarily (v2)

Updated part 1 patch as described above; carrying r+

Comment 12

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

Attachment: Part 8 - More code deletion from browser.js

There's more code deletion that can be done from browser.js. Still testing this locally to make sure I didn't screw it up.

Comment 13

[Botond Ballo [:botond]]

Comment on attachment 8655059 [details] [diff] [review]
Part 3 - Some cleanup in GetViewportInfo

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

If we're going to be maintaining this invariant ("the default value of allowDoubleTapToZoom is the same as the initial value of allowZoom"), then, given that no one calls nsViewportInfo::SetAllowDoubleTapZoom(), nor does anyone change nsViewportInfo::mAllowZoom, is there any value in storing nsViewportInfo::mAllowDoubleTapZoom in the first place?

Comment 14

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

(In reply to Botond Ballo [:botond] from comment #13)
> If we're going to be maintaining this invariant ("the default value of
> allowDoubleTapToZoom is the same as the initial value of allowZoom"), then,
> given that no one calls nsViewportInfo::SetAllowDoubleTapZoom(), nor does
> anyone change nsViewportInfo::mAllowZoom, is there any value in storing
> nsViewportInfo::mAllowDoubleTapZoom in the first place?

Yeah that's a excellent point, I can get rid of it.

Comment 15

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

Attachment: Part 9 - Remove nsViewportInfo::mAllowDoubleTapZoom

Comment 16

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0c678c16f473
https://hg.mozilla.org/integration/mozilla-inbound/rev/db5c0d802e67
https://hg.mozilla.org/integration/mozilla-inbound/rev/fb074ed8557f
https://hg.mozilla.org/integration/mozilla-inbound/rev/62aac0e570e3
https://hg.mozilla.org/integration/mozilla-inbound/rev/2b2a45987233
https://hg.mozilla.org/integration/mozilla-inbound/rev/730d2acfbdb6
https://hg.mozilla.org/integration/mozilla-inbound/rev/211782e11049
https://hg.mozilla.org/integration/mozilla-inbound/rev/6605bf7174c8
https://hg.mozilla.org/integration/mozilla-inbound/rev/ea9f2295023b

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/0c678c16f473
https://hg.mozilla.org/mozilla-central/rev/db5c0d802e67
https://hg.mozilla.org/mozilla-central/rev/fb074ed8557f
https://hg.mozilla.org/mozilla-central/rev/62aac0e570e3
https://hg.mozilla.org/mozilla-central/rev/2b2a45987233
https://hg.mozilla.org/mozilla-central/rev/730d2acfbdb6
https://hg.mozilla.org/mozilla-central/rev/211782e11049
https://hg.mozilla.org/mozilla-central/rev/6605bf7174c8
https://hg.mozilla.org/mozilla-central/rev/ea9f2295023b