1112740 - [sms] Thread list header "Messages" font size too small

Status: RESOLVED DUPLICATE of bug 1112131

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

Description

[Michael Henretty [:mikehenrty][:mhenretty]]

Attachment: [screenshot] tiny "messages"

Comment 1

[Julien Wajsberg [:julienw] (PTO -> Mar 16)]

Maybe it will be fixed by bug 1089920, but let's have a blocking status so that we don't lose it in case it's not.

Comment 2

[Johan Lorenzo [:jlorenzo]]

Triage: Visible regression.

Comment 3

[Julien Wajsberg [:julienw] (PTO -> Mar 16)]

What's funny is that I don't see this on my Buri or my Flame with a current master :)

QA, can you please test this, and possibly find a STR for this? Or does it happen only sometimes?

Comment 4

[Johan Lorenzo [:jlorenzo]]

Adding steps wanted as we don't currently have STRs.

Comment 5

[Cody Roesch [:croesch]]

This is a Full vs Shallow flash issue.

Shallow flashing the Flame device, I get this small text every time in both User Nightly and Engineering builds. However If I full flash a user nightly build then the Messages header is correct size.

Tested with Shallow Flash on 319mb using Engineering builds.

This bug repro's on Flame KK builds: Flame 2.2 KK, Flame 2.1 KK

Actual Results: Small Messages header seen when shallow flashing in the the branches below.

Repro Rate: 4/4

Environmental Variables:
Device: Flame 2.2 KK
BuildID: 20141223065136
Gaia: 0db8a38f9fed18ae2abf5ef7e1b6e2a570b07e0e
Gecko: bc71b49d9bf8
Version: 37.0a1 (2.2) 
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
-----------------------------------------------------------------
Environmental Variables:
Device: Flame 2.1 KK
BuildID: 20141222071650
Gaia: 1a58335917b23c170d85bf821f658e964c935b23
Gecko: 5509822354a9
Version: 34.0 (2.1) 
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

-----------------------------------------------------------------
-----------------------------------------------------------------

This bug does NOT repro on Flame kk build: Flame 2.0 KK, Flame v188-1 Base

Actual Result: Correct Size Messages header when shallow flashing.

Repro Rate: 0/2

Environmental Variables:
Device: Flame 2.0 KK
BuildID: 20141222071429
Gaia: ce83ea7b8e3fa2d1c3fd771fc22b654c18b3c381
Gecko: 7d0adaf6c447
Version: 32.0 (2.0) 
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
-----------------------------------------------------------------
Environmental Variables:
Device: Flame v188-1 Base
BuildID: 20141021162107
Gaia: 8c5c956ee6909408e29f375cc7d843a03d92f3d8
Version: 32.0 (2.0)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 6

[Joshua Mitchell (Inactive)]

Michael - do you know if you were Shallow flashing when you saw this issue?

If so - might be a dupe of (or a see-also) bug 1069495

Comment 7

[Michael Henretty [:mikehenrty][:mhenretty]]

Yup, I definitely shallow flash.

Comment 8

[Julien Wajsberg [:julienw] (PTO -> Mar 16)]

At least I think it's not a blocker anymore, let's renominate.

On my Buri I use the correct fonts and I don't see the issue.

I don't really understand why having bad fonts would trigger this issue though, unless we calculate using some fonts and display using other fonts?

Comment 9

[Julien Wajsberg [:julienw] (PTO -> Mar 16)]

Would be useful to know if you have the issue when first full flashing (and thus getting the right fonts), and then shallow flashing.

Thanks!

Comment 10

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Per comment 8 I agree this is not a blocker given the STR is not a usual case for end user.

Comment 11

[Michael Henretty [:mikehenrty][:mhenretty]]

(In reply to Julien Wajsberg [:julienw] (PTO 12/25 -> 12/29) from comment #8)
> I don't really understand why having bad fonts would trigger this issue
> though, unless we calculate using some fonts and display using other fonts?

We calculate the font using getComputedStyle for the header element. But perhaps if we don't have the right font installed, we get `unknown` here [1], which should mess up all font fit calculations and not just thread list.

1.) https://github.com/mozilla-b2g/gaia/blob/3042a3d27ccc5ed2ea7e40a33769369d797b06c3/shared/elements/gaia-header/lib/font-fit.js#L223

Comment 12

[Julien Wajsberg [:julienw] (PTO -> Mar 16)]

Right, maybe replacing "unknown" by "sans-serif" would be a good idea?

Comment 13

[Cody Roesch [:croesch]]

(In reply to Julien Wajsberg [:julienw] (PTO 12/25 -> 12/29) from comment #9)
> Would be useful to know if you have the issue when first full flashing (and
> thus getting the right fonts), and then shallow flashing.
> 
> Thanks!

Julien,

After Basing then full flashing (verify correct size) then shallow flashing, I found the font size to retain correct behavior. This was seen on the 12/29 Nightly build.

Environmental Variables:
Device: Flame 2.2 Master (319mb)(Kitkat Base
Build ID: 20141229010215
Gaia: bdedbaf9f18a43c091ede770407d68d38582fe29
Gecko: 8850aa0f5332
Gonk: a814b2e2dfdda7140cb3a357617dc4fbb1435e76
Version: 37.0a1 (2.2)
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Comment 14

[Michael Henretty [:mikehenrty][:mhenretty]]

(In reply to Julien Wajsberg [:julienw] (PTO 12/25 -> 12/29) from comment #12)
> Right, maybe replacing "unknown" by "sans-serif" would be a good idea?

Worth a try.

Comment 15

[Julien Wajsberg [:julienw] (PTO -> Mar 16)]

Moving to Gaia::Component, we'll land the simple change outlined in comment 12 here.

Comment 16

[Julien Wajsberg [:julienw] (PTO -> Mar 16)]

To a contributor: we want to change the code in [1]: replace 'unknown' by 'sans-serif'. You'll need to update the unit tests if this makes sense (I'm not sure it does).

[1] https://github.com/gaia-components/gaia-header/blob/2fffa7fcfb842399fcce5cc9d915988ddc901269/lib/font-fit.js#L230

Comment 17

[awalgarg]

Hi there,

I am new here, and I would like to take this as my first bug :) I have created the initial commit with the PR here: https://github.com/gaia-components/gaia-header/pull/17

awalGarg

Comment 18

[Julien Wajsberg [:julienw] (PTO -> Mar 16)]

Hi, awalgarg, thanks, looks good !

Can you try to add a test in test/font_fit_test.js ?
I don't see how to do it otherwise than calling directly _getStyleProperties with a heading having a bogus fontFamily. If this don't work, please keep it in the pull request and ask me so that I can have a look :)

You have information about how to run tests in [1].

In case you use Linux, I personally need to add "FIREFOX_NIGHTLY_BIN=/path/to/firefox" before "npm test" so that it correctly picks up my Firefox Nightly, but if you use MacOS X I think it works automatically.

[1] https://github.com/gaia-components/gaia-header/#tests

Thanks !

Comment 19

[awalgarg]

Hi Julien,

I would add a test in a while. Yep, I am using Linux, I would keep that in mind.

Thanks for the guidance!

awalGarg

Comment 20

[awalgarg]

Hi Julien,

Sorry I am too late... I have added two tests. Kindly review here: https://github.com/gaia-components/gaia-header/pull/17

Thanks

awalGarg

Comment 21

[Julien Wajsberg [:julienw] (PTO -> Mar 16)]

The patch in bug 1112131 will change a lot how the gaia-header component works and it should fix this issue as well.

Comment 22

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

removing the nom as it's dup is already set as blocking-b2g:2.2+.