1124305 - [Notification Menu][Music Widget] Descenders (letters with 'tails') are slightly cut off when viewing song titles on the music widget on the notification menu.

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System::Window Mgmt, defect)

Description

[Joshua Mitchell (Inactive)]

Attachment: 2015-01-21-10-10-48.png

Description:
When you have music playing through the music app and access the notification menu there will be a music widget present (just above the usage tracking section). On this widget letters with descenders are cut off on the bottom. This is MOST noticeable with the letter g, but also apparent with y,p,q and j 
This ALSO happens on the music widget that appears on the lockscreen

Repro Prereq: Have music on device with lower case g in title

Repro Steps:
1) Update a Flame to 20150121010204
2) Launch Music App and play a song with lower case g in title
3) Access notification menu

Actual:
lower case descenders are cut-off at the bottom

Expected:
letters will appear complete

Environmental Variables:
Device: Flame 3.0 (KK - Nightly - Full Flash)
Build ID: 20150121010204
Gaia: 5e98dc164b17fd6decb48a9eaddef0e55b82e249
Gecko: 540077a30866
Gonk: a814b2e2dfdda7140cb3a357617dc4fbb1435e76
Version: 38.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

Repro frequency: 5/5
See attached: screenshot

--------------------------------------------------------------------------------------------------
This also occurs on Flame 2.2 (v18d-1), 2.2 (v18d), 2.1 (v18d-1),


Device: Flame 2.2 (KK - Nightly - Full Flash)
Build ID: 20150121002607
Gaia: e4f9b5da3751798f9cc5d95f302c30722cc11fca
Gecko: 75a462a58d7a
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0a2 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Device: Flame 2.2 (KK - Nightly - Full Flash)
Build ID: 20150120002507
Gaia: f5b3d1b6cfa3e702033f613915ae637cb735cbfb
Gecko: 5d7497ce4cc7
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0a2 (2.2)
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Device: Flame 2.1 (KK - Nightly - Full Flash)
Build ID: 20150120001202
Gaia: 77c57eb8a985d5cbd34a597fb1b978ba6e205af6
Gecko: f05d0a2d2378
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 34.0 (2.1)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
---------------------------------------------------------------------------------

This issue does NOT appear in 2.0

Actual results: descender letters are not cut-off

Device: Flame 2.0 (KK - Nightly - Full Flash)
Build ID: 20150120000224
Gaia: 736933b25ded904f0cb935a0d48f1f3cf91d33ad
Gecko: 6ff30f9ba931
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 32.0 (2.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 1

[Peter Bylenga [:PBylenga]]

Not nominating to block, minor graphical issue.  It is a regression, nominating for polish work.

Comment 2

[Hermes Cheng (inactive after July 27, 2015)]

NI System developer

Comment 3

[(inactive after 6/18) Alive Kuo [:alive]]

Need UX input. Switch ni to dkuo.

Comment 4

[Stephany Wilkes]

Assigning to Tif for music.

Comment 5

[Tiffanie Shakespeare [:tif]]

Did something change between the two releases that would cause this regression? Otherwise, can we not just make it how it was?

Comment 6

[Dominic Kuo [:dkuo]]

Attachment: Music_playback_widget.png

Comment 7

[Dominic Kuo [:dkuo]]

Looks fine on my flame with master build, according to attachment 8569072 [details].

Comment 8

[Tiffanie Shakespeare [:tif]]

Attachment: Text cut off in music player

Odd - I just checked it on my flame and I still see the issue.

Gaia-Rev        7894b929f1b0394f3c997f72a6482bc7813e758d
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/0a8b3b67715a
Build-ID        20150225010244
Version         39.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150225.043702
FW-Date         Wed Feb 25 04:37:14 EST 2015
Bootloader      L1TC00011880

Comment 9

[Hermes Cheng (inactive after July 27, 2015)]

Attachment: screenshot for build 20150310162504

Use below engineer build and still has this issue.

Build ID               20150310162504
Gaia Revision          5af6f8d5d6161dea02002634c6d0a570a122e5dd
Gaia Date              2015-03-10 19:17:12
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/ec87adb8cf13
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150310.200728
Firmware Date          Tue Mar 10 20:07:39 EDT 2015
Bootloader             L1TC000118D0

Comment 10

[GitHub Autolander Bot]

Attachment: [gaia] dominickuo:bug-1124305 > mozilla-b2g:master

Comment 11

[Dominic Kuo [:dkuo]]

Though I cannot reproduce this bug on my flame with master build, but I traced the music widget styles, I found the current css could possibly cause this issue so I tried to modify them in a proper way, and patch attached but I need some helps to confirm the patch does work.

Comment 12

[Dominic Kuo [:dkuo]]

Comment on attachment 8579903 [details] [review]
[gaia] dominickuo:bug-1124305 > mozilla-b2g:master

Tif, would you please check my patch and see if it works(since you are able to repro it)? thanks!

Comment 13

[Dominic Kuo [:dkuo]]

or Hermes could you please help on testing the patch? since you ni me and you should know how to reproduce it, thanks.

Comment 14

[Hermes Cheng (inactive after July 27, 2015)]

Attachment: notification with patch.png

Hi Dominic, I tried your patch but still got this issue. Beside, it also exists on lockscreen music playback.

Comment 15

[Hermes Cheng (inactive after July 27, 2015)]

Attachment: lockscreen with patch.png

It is not really clear because of the background color, but it is the same issue on Lockscreen.

Comment 16

[Tiffanie Shakespeare [:tif]]

Sorry for the delay! Unfortunately, I still see the issue :-/

Here's what I used to test.

Build ID               20150323010204
Gaia Revision          c44790956c685d4e0eaef629dad212d7b4fdd7a0
Gaia Date              2015-03-19 09:14:52
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/e730012260a4
Gecko Version          39.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150313.041854
Firmware Date          Fri Mar 13 04:19:06 EDT 2015
Bootloader             L1TC00011880

Comment 17

[Hermes Cheng (inactive after July 27, 2015)]

Dominic, could you take a look? Thanks. ;)

Comment 18

[Dominic Kuo [:dkuo]]

While I was waiting the test result, I found the necessary step to reproduce this, that's, I have to flash the image file instead of replacing gaia+gecko only, it's probably the font files but anyway, I am able to reproduce this and should have a patch later.

Comment 19

[Dominic Kuo [:dkuo]]

Comment on attachment 8579903 [details] [review]
[gaia] dominickuo:bug-1124305 > mozilla-b2g:master

Okay, this should fix both the music playback widgets in utility tray and lockscreen.

Jim, would you please review this?

Tif, if you get a chance to test the updated patch again, the issue should be gone with it and please let me know if it's not.

Thanks all!

Comment 20

[Jim Porter (:squib)]

Comment on attachment 8579903 [details] [review]
[gaia] dominickuo:bug-1124305 > mozilla-b2g:master

I suppose this is ok, although it seems kind of complex for something that should be really simple. I'd recommend having a system/lockscreen reviewer look at this too.

Comment 21

[Dominic Kuo [:dkuo]]

(In reply to Jim Porter (:squib) from comment #20)
> Comment on attachment 8579903 [details] [review]
> [gaia] dominickuo:bug-1124305 > mozilla-b2g:master
> 
> I suppose this is ok, although it seems kind of complex for something that
> should be really simple. I'd recommend having a system/lockscreen reviewer
> look at this too.

The root cause is, the track title's |line-height| and |font-size| are set to a same value(1.4rem), this caused issues because theoretically line-height includes text height and row spacing, so by default it should be larger than the font-size if line-height is not set, but we set it wrong.

So I removed the line-height style and let gecko calculate for us, then I found the previous author tried to use padding-top to align the track title to bottom, it's also not a proper way to do that. And to keep the div original height I use |display: table-cell| and |vertical-align: bottom| to solve the align bottom problem, and that's why the patch looks not-so-simple but with two corrections.

Comment 22

[Dominic Kuo [:dkuo]]

Comment on attachment 8579903 [details] [review]
[gaia] dominickuo:bug-1124305 > mozilla-b2g:master

Alive, would you please review this patch? thanks. (comment 21 explained why the patch looks like that)

Comment 23

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8579903 [details] [review]
[gaia] dominickuo:bug-1124305 > mozilla-b2g:master

css only change, r=me

Comment 24

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/d41d4c4e475f2439001a3f6a5321cb81106b4fe7

Comment 25

[Shally Li（leave from Mozilla）]

According to the STR of Comment 0,this bug has been successfully verified on latest Nightly Flame v3.0.
See attachment: verified_v3.0.png
Reproduce rate: 0/5

Device: Flame 3.0 build(Pass)
Build ID               20150326160206
Gaia Revision          525c341254e08f07f90da57a4d1cd5971a3cc668
Gaia Date              2015-03-26 16:34:16
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/59554288b4eb
Gecko Version          39.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150326.193247
Firmware Date          Thu Mar 26 19:32:58 EDT 2015
Bootloader             L1TC000118D0

Leaving "verifyme" keywords for Flame v2.1&2.2.

Comment 26

[Shally Li（leave from Mozilla）]

Attachment: verified_v3.0.png

Comment 28

[howie [:howie]]

Blocking as regression.

Comment 29

[Dominic Kuo [:dkuo]]

Comment on attachment 8579903 [details] [review]
[gaia] dominickuo:bug-1124305 > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Bug 1038723.
[User impact] if declined: Bad ux with song titles cut off.
[Testing completed]: CSS changes only so no tests needed, but verified with human eyes.
[Risk to taking this patch] (and alternatives if risky): Low.
[String changes made]: None.

Comment 30

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/194c502a23e04a9bfbc4c34d3983687e8b2ab527

Comment 32

[Pi Wei Cheng [:piwei] (inactive)]

Attachment: verified screenshots on english and arabic

This issue is verified fixed on 3.0 and 2.2. Because of bug 1148241 being duped to this I had to check that this is fixed in Arabic, and indeed it is.

See attached screenshots for verified behavior.

Device: Flame 3.0 Master (full flash 319MB mem)
BuildID: 20150401010204
Gaia: 03164bd160809747e6a198e0dba1b7c3ee7789f5
Gecko: 18a8ea7c2c62
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 40.0a1 (3.0 Master)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

Device: Flame 2.2
BuildID: 20150401002624
Gaia: 8b3086ad3963f1707e2bee9094baccafffe161c4
Gecko: 20b67213a047
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
Version: 37.0 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0