841760 - Lazy load more things in the settings app.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Settings, defect)

Description

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

Attachment: CSS Patch

This will be divided in 2 parts. One for the css, one for the js.

Comment 1

[[:fabrice] Fabrice Desré]

That should probably not block a *gecko* bug :P

Comment 2

[Etienne Segonzac (:etienne)]

Attachment: JS Part

Spoiler:

with the "optimize" build step activated and the wing in the right direction this patch on top of the CSS one get's us a settings startup time < 1sec :)

Comment 3

[Etienne Segonzac (:etienne)]

s/wing/wind

Comment 4

[Etienne Segonzac (:etienne)]

Comment on attachment 714378 [details] [diff] [review]
CSS Patch

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

::: apps/settings/js/settings.js
@@ +132,5 @@
> +    // activate all stylesheets
> +    var stylesheets = panel.querySelectorAll('link');
> +    for (var i = 0; i < stylesheets.length; i++) {
> +      var href = stylesheets[i].getAttribute('href');
> +      if (document.head.querySelector('link[href="' + href + '"]'))

wouldn't mind some curly braces

Comment 5

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

Etienne will merge it so assigning to him :) (thanks!)

Comment 6

[Anshul]

Should this be uplifted to v1.0.1?

Comment 7

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: Part 1 rebased to 1.0.1

Comment 8

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 714703 [details] [diff] [review]
Part 1 rebased to 1.0.1

Looks like these patches conflict a bit, so I'll let you guys sort it out :).

Comment 9

[Lucas Adamski [:ladamski]]

Should be pretty safe, mostly refactoring & renaming.

Comment 10

[Lucas Adamski [:ladamski]]

Apologies for the churn.  Verified with akeybl that perf bugs we want to land should be nominated for TEF+ vs simply asking for approval.

Comment 11

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

Attachment: Patch - delay more things.

There is no need to block the page load on those scripts - let's show the page content as soon as possible.

Comment 12

[Etienne Segonzac (:etienne)]

Comment on attachment 716416 [details] [diff] [review]
Patch - delay more things.

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

Looks like it's breaking stuffs.
Let me know if you need somebody else to debug it.

Comment 13

[Alex Keybl [:akeybl]]

We'll have to resolve this in a future release. Too much risk a couple of days before being totally code frozen for v1.0.1.

Comment 14

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

Attachment: Patch

Sigh. The regression are because I forgot a .js at the end of connectivity.js...

Comment 15

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

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

Comment 16

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

Since we are tef+ some bugs that does not change much I feel like a few hundreds of milliseconds worth tef+'ing it.

Comment 17

[John Ford [:jhford] CET/CEST Berlin Time]

Commit ef60a5e50ef39c7fd15cb07dbdfaa1a23cd7d747 does not apply to v1-train or v1.0.1.  This means that there are merge conflicts which need to be resolved.  If there are dependencies that are not approved for branch landing, or have yet to land on master, please let me know

If a manual merge is required, a good place to start might be:
  cd gaia
  git checkout v1-train
  git cherry-pick -x -m1 ef60a5e50ef39c7fd15cb07dbdfaa1a23cd7d747
  <RESOLVE MERGE CONFLICTS>
  git checkout v1.0.1
  git cherry-pick -x $(git log -n1 v1-train)

Comment 18

[Julien Wajsberg [:julienw]]

Actually, there were 3 commits on master :

2f86aad75e00ffd578f8f5245fcde81af2ef516d
bb65b9c82bade667a07bc699bdbd5c80daa44df3
ef60a5e50ef39c7fd15cb07dbdfaa1a23cd7d747

It would make sense to land the first two at once, and the third one separatly, because they landed at 10 days interval.

It still doesn't apply cleanly yet but I'm on it.

Comment 19

[Julien Wajsberg [:julienw]]

This doesn't apply at least because of Bug 832082 that should (maybe) land on v1-train/v1.00.1 too, so I'm waiting for an answer there first.

Comment 20

[John Ford [:jhford] CET/CEST Berlin Time]

v1-train: ad21b1adce731abe47bce6518d29b272dbb8faa3
v1-train: 07b7f216a9a7d5a0a7acaa576a950b996158d1d8
v1-train: 7d1cdb5a11fbd47ec1d8ac131b9330120e293f60

On v1.0.1, only 2f86aad75e00ffd578f8f5245fcde81af2ef516d applies cleanly, but I won't uplift anything there yet because I assume we want these atomically applied.

Comment 21

[Daniel Coloma:dcoloma]

Marking v1.0.1 as affected so we can track this

Comment 22

[Julien Wajsberg [:julienw]]

v1.0.1: 8164523544e500e388d85d546bc51672b8e0d6b0

(pushed the first one only by error, doing the others right now)

Comment 23

[Julien Wajsberg [:julienw]]

v1.0.1: 37ad710061197f5818fa6afd010b698232dda7c9
v1.0.1: 60cade2203b8237ce3d7aedcd759a537029eb8cb

Comment 24

[amiller]

does not make sense to create a regression issue.

Comment 25

[Cody Roesch [:croesch]]

Can you please provide steps to verify this fix - as we will blackbox test from the UI?

Comment 26

[Julien Wajsberg [:julienw]]

Nothing special, just check that everything works ok, including navigating to subpanels and manipulating controls, eg changing the language.

Comment 27

[Darren]

Cannot verify, need specifics on what exactly was changed so we know what to check.

Comment 28

[John Hammink]

Check that the settings app work as per testruns and smoketests.