929728 - Add a setting to enable APZC in all of gaia

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

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

APZC can be turned on in all gaia apps now, but there are some issues in various apps that need to be fixed (see some of the other bugs blocking 909877). In order to allow gaia developers to more easily investigate these problems, Vivien suggested adding a debug option to enable APZC everywhere.

Patches incoming.

Comment 1

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

Attachment: Part 1 - Gecko patch to read and apply the settings change

Comment 2

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

Attachment: Change to gaia settings app

Comment 3

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

Note that only apps that are created *after* checking the box will have the APZC enabled. You will need to restart b2g to enable APZC on the homescreen app.

Comment 4

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 820629 [details] [diff] [review]
Part 1 - Gecko patch to read and apply the settings change

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

Instead of having both a Gecko patch and a Gaia patch you can do all the work in Gaia.

Just add a SettingsListener in apps/system/js/browser_frame.js that toggle a global variable (for this file) that force the 'asyncpanzoom' attribute to be set.

Comment 5

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

Comment on attachment 820631 [details] [review]
Change to gaia settings app

Updated to be all in gaia. I assume it's expensive to call SettingsListener.getSettingsLock().get(apzSetting) otherwise I could just do that on every frame creation instead of storing a local variable.

Comment 6

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 820631 [details] [review]
Change to gaia settings app

Can you add a comment explaining that it is temporary?

Also it is not really expensive, but just async so that would delay frames creation.

Comment 7

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

I updated the PR with the comment. I also noticed that the travis-CI build ran on my PR and failed the unit tests for browser_frame.js. I patched that up too but I haven't been able to verify that my fixes to that work - the travis CI failed for some unrelated reason it looks like, and I haven't been able to successfully run the tests locally.

Comment 8

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

I re-pushed the PR and the unit test for browser_frame.js is still failing. I need to figure out how to run the test locally so I can fix it without round-tripping through travis all the time.

Comment 9

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

Comment on attachment 820631 [details] [review]
Change to gaia settings app

Ok, with Rik's help I was able to repro the failure locally and fix it. The fix was a little more involved than I thought it would be, but I tried to make it as elegant as I could. PR is updated with the fix, but re-requesting review since the fix to the test was nontrivial.

Feel free to merge the PR if the travis run is green and you're fine with the changes I made.

Comment 10

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

The travis run failed but for what looks like an unrelated thing.

Comment 11

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

https://github.com/mozilla-b2g/gaia/commit/ac5748783091023f7f7b010461d767d5cc6fc321