902390 - [sms][mms] the input is 1px too high with the 'Moz' font

Status: RESOLVED FIXED

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

Description

[Julien Wajsberg [:julienw]]

STR:
* open the SMS app
* start a the new message, and press the first character
=> notice the 1px difference

This happens since we switched to the MozTT font by default, because with the old font this is not happening

The fix is very easy: just put a min-height to 2.3rem (because the height with a character with this font is 23px) (tried with Firefox, not yet on the device).

The additional problem with this bug is that it's triggering a synchronous reflow because of this.

Asking leo? because it's a no-risk patch for a noticeable improvement for the user.

Comment 1

[Julien Wajsberg [:julienw]]

Attachment: patch v1

Made the min-height change from 1.9rem to 2.3rem.

Comment 2

[Julien Wajsberg [:julienw]]

Comment on attachment 786849 [details] [diff] [review]
patch v1

Corey, if you want to take this review, feel free :)

Comment 3

[Corey Frang [:gnarf]]

I assume you tested this on the device and it fixed the problem.  Before/After screenshot?

Comment 4

[Borja Salguero [:borjasalguero]]

Julien, I would like to see this issue in the device as well. Tomorrow morning we could take a look directly on the device and check the behaviour! :)

Comment 5

[Borja Salguero [:borjasalguero]]

Comment on attachment 786849 [details] [diff] [review]
patch v1

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

I've been testing this in the device and now I can see the issue! This patch is working as expected. R+ for sure!

Comment 6

[Julien Wajsberg [:julienw]]

thanks :)

master: 54e79c453515a7b2ad443ccd99936ced1b3ff029

Corey: we don't see this so well with the screenshots because 1px is small and it's easier to see it change than comparing 2 screenshots, therefore it's far more easier to test it directly on the device.

Comment 7

[Lukas Blakk [:lsblakk] use ?needinfo]

Can UX team please speak to this?  When did we switch to MozTT font as default? Is that a v1.1 blocker?

Comment 8

[Patryk Adamczyk [:patryk] UX]

We switched to MozTT as a default for v.1.1. 
MozTT (now renamed Fira Sans), should be our default font everywhere. The issue was that Open Sans was default and we over wrote it in most places by specifying MozTT, this caused issues with certain apps displaying mixed fonts.

Comment 9

[Julien Wajsberg [:julienw]]

Lucas: this is not a blocker per se, but the fix is so small it's imho worth it to include it in 1.1. Without the fix the user will see a small "jump" when he's writing the first letter of a sms. This jump will triggers a workflow as well, and because of some other code, that workflow is synchronous, which adds a lag when typing that first letter.

Comment 10

[Stephany Wilkes]

Adding some context here. Font issues should not block 1.1 but should block 1.1 hd+ and later.

I marked this bug as depending on 897009, which fixes a majority of font issues, though this bug looks like a design problem. Since v.1.0 MozTT (formerly known as Feura Sans) should have been the default font in every app. Some devs either did not know this or ignored it, and used Open Sans instead. We fixed many of these issues for v.1.1 but looks like with the redesign of the SMS app, Open Sans has surfaced again.

There *was* a change to make MozTT the default font everywhere, which we think caused some issues. The new font, MozTT, has many new features and languages but sizing should be identical.

Comment 11

[Gabriele Svelto [:gsvelto]]

(In reply to Julien Wajsberg [:julienw] from comment #9)
> This jump will triggers a
> workflow as well, and because of some other code, [...]
  ^^^
  (layout) reflow :)

It can be pretty bad on long threads giving an unresponsive feeling to the user when he starts to type a new message.

Comment 12

[Julien Wajsberg [:julienw]]

(In reply to Stephany Wilkes from comment #10)
> Adding some context here. Font issues should not block 1.1 but should block
> 1.1 hd+ and later.

Stephany, what is the rationale for blocking 1.1hd but not 1.1 ? Does the new font bring something special for such devices ?

Comment 13

[Stephany Wilkes]

Julien, I need Patryk to speak to that: the date I got was from him.

Comment 14

[Alex Keybl [:akeybl]]

Emailed Stephany to clear this up.

Comment 15

[Stephany Wilkes]

Needinfo? to Patryk to confirm and clarify.

Comment 16

[Patryk Adamczyk [:patryk] UX]

The fonts add the following which are known bugs on v.1.0
+ full support for Cyrillic
+ full support for Greek

Currently we a user switches to those languages not all characters appear.

If there is a chance to get this into v.1.1, then YES lets do it.
There is definitely time to get this into v.1.1 HD.

Comment 17

[Lukas Blakk [:lsblakk] use ?needinfo]

Seems low risk and high impact, we can block and take this on both v1.1 and v1.1hd

Comment 18

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

Uplifted 54e79c453515a7b2ad443ccd99936ced1b3ff029 to:
v1-train: 23a8ee5231e010076478dce9fc7f9f359341cf39

Comment 19

[Tim Guan-tin Chien [:timdream] (please needinfo)]

v1.1.0hd: 23a8ee5231e010076478dce9fc7f9f359341cf39

Comment 20

[Sarah Parsons]

Verified fixed on the Leo device Build ID: 20130816041203

Build ID: 20130816041203
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/09a8c5b6b287
Gaia: 6040508a535b587d4155356a47b22d7bd9ea4de7
Platform Version: 18.1
RIL Version: 01.01.00.019.164