1218752 - [Music][NGA] Font-fit is not running on header titles

Status: RESOLVED WONTFIX

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

Description

[Wilson Page [:wilsonpage]]

Attachment: index.png

We currently have the `no-font-fit` attribute on <gaia-header>. I assume this was a performance optimization for startup.

Although this attribute is never being removed. This means that we're never sizing title text to fit (see attached screenshot).

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] ssaavedra:bugfix-1218752-music-no-font-fit > mozilla-b2g:master

Comment 2

[Santiago Saavedra]

I think this fixes it, but I could not test it on real hardware yet, and I could not make the simulator find the music files I have.

Comment 3

[Alberto Pastor [:albertopq]]

Assigning. Thanks Santiago!

Comment 4

[Wilson Page [:wilsonpage]]

Let's ask Justin why the `not-font-fit` attribute was being used in the first place.

Comment 6

[Justin D'Arcangelo [:justindarc]]

(In reply to Wilson Page [:wilsonpage] from comment #4)
> Let's ask Justin why the `not-font-fit` attribute was being used in the
> first place.

At one point during development, I added this to address an issue where the text in the header would shift left/right depending on what header buttons were visible at the time. I remember that it was visually distracting, but I can't remember what the exact STR was. Its also worth mentioning that the OGA Music app also had this attribute in its <gaia-header>, presumably for similar reasons.

Comment 7

[Jim Porter (:squib)]

OGA Music removes "no-font-fit" immediately after the DB is enumerable. It's strictly a performance optimization.

Comment 8

[Santiago Saavedra]

(In reply to Jim Porter (:squib) from comment #7)
> OGA Music removes "no-font-fit" immediately after the DB is enumerable. It's
> strictly a performance optimization.

But that seems not to be the case in NGA, right?

(In reply to Justin D'Arcangelo [:justindarc] from comment #6)
> At one point during development, I added this to address an issue where the
> text in the header would shift left/right depending on what header buttons
> were visible at the time. I remember that it was visually distracting, but I
> can't remember what the exact STR was. Its also worth mentioning that the
> OGA Music app also had this attribute in its <gaia-header>, presumably for
> similar reasons.

But it seems the attribute is not being removed by any logic in NGA.

Should then we add logic to NGA instead of just letting the style be computed in the gaia-header component?

Comment 9

[Jim Porter (:squib)]

(In reply to Santiago Saavedra from comment #8)
> (In reply to Jim Porter (:squib) from comment #7)
> > OGA Music removes "no-font-fit" immediately after the DB is enumerable. It's
> > strictly a performance optimization.
> 
> But that seems not to be the case in NGA, right?

Correct.

> Should then we add logic to NGA instead of just letting the style be
> computed in the gaia-header component?

I think we should try to make gaia-header more performant in this case if possible. Users of gaia-header shouldn't have to perform subtle tricks like OGA music does to boost performance. Overall, your patch looks good, and if we need to change anything more, I think gaia-header would be the best place to do it.

Comment 10

[Santiago Saavedra]

(In reply to Jim Porter (:squib) from comment #9)
> I think we should try to make gaia-header more performant in this case if
> possible. Users of gaia-header shouldn't have to perform subtle tricks like
> OGA music does to boost performance. Overall, your patch looks good, and if
> we need to change anything more, I think gaia-header would be the best place
> to do it.

Then, should my patch be "the correct way" of doing it in the music app?

Comment 11

[Wilson Page [:wilsonpage]]

(In reply to Jim Porter (:squib) from comment #9)
> I think we should try to make gaia-header more performant in this case if
> possible. Users of gaia-header shouldn't have to perform subtle tricks like
> OGA music does to boost performance.

Not really sure what you mean by this?

> Overall, your patch looks good, and if we need to change anything more, I think gaia-header would be the best place to do it.

Please r+ and land this patch if you're happy with it :)

Comment 12

[Wilson Page [:wilsonpage]]

> I think we should try to make gaia-header more performant in this case if
> possible. Users of gaia-header shouldn't have to perform subtle tricks like
> OGA music does to boost performance.

In the case of Music the reason you are able to not run font-fit on startup is because you are sure that the header title of the first view will never overflow. This is app specific and not a decision gaia-header can make.

Comment 13

[Jim Porter (:squib)]

Comment on attachment 8690330 [details] [review]
[gaia] ssaavedra:bugfix-1218752-music-no-font-fit > mozilla-b2g:master

Justin would probably be a better person to ask about this, since he probably cares more about startup perf. :)

I for one don't have any problems with this patch, although I'd also be ok with doing something like OGA too (i.e. removing no-font-fit after the initial load).

Comment 14

[Justin D'Arcangelo [:justindarc]]

Comment on attachment 8690330 [details] [review]
[gaia] ssaavedra:bugfix-1218752-music-no-font-fit > mozilla-b2g:master

Just going through and clearing old stuff out of my queue. I'm giving an R- here because I believe we did remove the font fit for perf reasons. The proper fix here (if we wanted to keep font fit) would be to wait until the critical startup path is finished before programmatically removing the [no-font-fit] attribute.

Comment 15

[BMO Automation]

Firefox OS is not being worked on