Closed Bug 1198132 Opened 6 years ago Closed 6 years ago

Apply gaia-header tweaks to Gallery to avoid reflow at startup

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(feature-b2g:2.5+, b2g-master fixed)

RESOLVED FIXED
FxOS-S6 (04Sep)
feature-b2g 2.5+
Tracking Status
b2g-master --- fixed

People

(Reporter: ting, Assigned: ting)

References

Details

Attachments

(1 file)

From the profile in bug 1196586 comment 2, gaia-header.js takes time doing font fit (~100ms on Flame) when launch Gallery, we can apply gaia-header tweaks [1] to avoid that.

[1] https://github.com/gaia-components/gaia-header
With attachment 8652187 [details] [review], on Flame the visullayLoaded gets ~80ms faster (1218ms to 1140ms).

The first time removing "no-font-fit" attribute takes ~50ms if there's a reflow, and ~20ms for header has both attributes title-start and title-end.
Attachment #8652187 - Flags: review?(dflanagan)
Comment on attachment 8652187 [details] [review]
[gaia] janus926:bug-1198132 > mozilla-b2g:master

Ting-Yu,

Thank you for working on this. Overall this patch looks fine. I'd slightly prefer to have the removeAttribute() code in startup.js instead of in the setView() function, however, so am giving r- for that reason. If you disagree, let's continue the discussion here with needinfos.

Also, I'm a little surprised to learn that gaia-header components slow down startup even though none of them are visible when the app starts. I wonder if there is any way that the component could do the font-fit algorithm when it first becomes visible, rather than when it is created.  I can't think of how thaat would work, though, so maybe this is not possible.  If I was refactoring the gallery today, I'd probably create each of the views (and headers) on demand, rather than loading the entire html file at startup, and we wouldn't have this problem.

See my comments on github.
Attachment #8652187 - Flags: review?(dflanagan) → review-
Blocks: 1180696
feature-b2g: --- → 2.5+
Assignee: nobody → janus926
Comment on attachment 8652187 [details] [review]
[gaia] janus926:bug-1198132 > mozilla-b2g:master

Addressed, thank you for your detailed comments :)
Attachment #8652187 - Flags: review- → review?(dflanagan)
Comment on attachment 8652187 [details] [review]
[gaia] janus926:bug-1198132 > mozilla-b2g:master

Please add a comment before the code that removes the attributes to explain what it is doing. And consider using for/of instead of [].forEach.call.

It looks good to me. Please land it when you're ready.

Note that I have not tested the code myself. I know that you have measured the startup time improvement, so I assume you've tested it and that the headers still work even when switching locales (between English and Runtime Accented, for exmample)
Attachment #8652187 - Flags: review?(dflanagan) → review+
(In reply to David Flanagan [:djf] from comment #5)
> Please add a comment before the code that removes the attributes to explain
> what it is doing. And consider using for/of instead of [].forEach.call.

Comment added; use for(;;) instead.

> Note that I have not tested the code myself. I know that you have measured
> the startup time improvement, so I assume you've tested it and that the
> headers still work even when switching locales (between English and Runtime
> Accented, for exmample)

Tested with different locales and the headers still work.

Thanks for reviewing :)
Keywords: checkin-needed
Master: https://github.com/mozilla-b2g/gaia/commit/d55f3327336b3bf3d7b0a9fe2020e968ac0e7a5b
Status: NEW → RESOLVED
Closed: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S6 (04Sep)
You need to log in before you can comment on or make changes to this bug.