950783 - [music] Progress bar in Music app is getting displayed in half of the screen.

Status: RESOLVED FIXED

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

Description

[Inder]

Steps to reproduce:
1.Go to Music app.
2.Start playback.
3.Progress bar is getting displayed within half of the screen.

Comment 1

[Hema Koka [:hema]]

Inder,

Could you please add info on the build and device please? And a screenshot if you can

Thanks
Hema

Comment 2

[Inder]

Attachment: progress_bar_half.png

screenshot attached.
device: qcom reference device.
build: Happening on latest v1.3/master code.
I have:
gecko:6e22321e2993507b583c237ef8d27b2b4778e7cb
gaia:1752e9e8f2b84b9db5d96ae5940596957fc8ed6c

Comment 3

[Dominic Kuo [:dkuo]]

I don't know the resolution but this should be a regression that caused by bug 939139, music app used to use % on the width so that it can fit to any devices, but it changed to use rem which only fits 480 x 320.

The patch should be simply revert the width from rem to %.

Comment 4

[Jim Porter (:squib)]

I knew I shouldn't have trusted the rem values when I reviewed that patch...

Taking this. It should be easy enough to fix.

Comment 5

[Jim Porter (:squib)]

Attachment: Fix this

Here we go. I tested this with a few resolutions in Firefox Desktop and everything looks ok.

Comment 6

[Marcia Knous [:marcia]]

Since this affects the reference device only the Media triage team does not consider this a blocker for 1.3. Removing blocking flag for now - please reply if you strongly disagree.

Comment 7

[Inder]

:marcia -- our testers have raised this as an issue so we need it fixed for v1.3

Comment 8

[Inder]

also from UX perspective this is not good so we should definitely fix it for 1.3

Comment 9

[Hema Koka [:hema]]

The fix is relatively straightforward and also it looks bad on the firefox desktop where we were able to reproduce. none of us have the QRD device that was used for testing it. adding fabrice since he is sheriffing bug fixes going into 1.3 for approval

marcia, we should try it on a helix to see if we can reproduce it (NI'd you for testing)

Comment 10

[Marcia Knous [:marcia]]

Not able to reproduce on Helix device using:

Gaia   a99249a7fdf9f20850d98a9a222385676d472362
SourceStamp 809aabadac6d
BuildID 20131219004002
Version 28.0a2

Comment 11

[Dominic Kuo [:dkuo]]

Comment on attachment 8349232 [details] [review]
Fix this

Jim,

Thanks for working on this, the patch looks good to me, there is one minor issue that probably needed to be addressed before we land it, please see github comments for details.

Comment 12

[Preeti Raghunath(:Preeti)]

Jim

Can you please land this patch?

Comment 13

[Jim Porter (:squib)]

I'm still looking into some issues Dominic raised on Github.

Comment 14

[Michael Vines [:m1] [:evilmachines]]

Hey Inder, can you verify that the patch fixes the issue here?

Comment 15

[Jim Porter (:squib)]

Dominic: Do you think it'd be safer to use 33% 34% 33% for the widths instead of calc(100% / 3)? I can change it, although I think calc() might be better since it would presumably give us a whole number for devices whose width is a multiple of 3 (I think HiDPI is like that).

Comment 16

[Dominic Kuo [:dkuo]]

I quickly experimented on "33% 34% 33%" and "calc(100% / 3)" with the resolution 320x480, and used DOM and Style inspector in nightly to see the computed widths of the three buttons. The results are kind of tricky but probably can explain my concern.

Both results are in pixel and the values show the computed styles of previous, play and next buttons.
1. "33% 34% 33%": 105.6 + 108.8 + 105.6 = 320
2. "calc(100% / 3)": 106.667 + 106.667 + 106.667 = 320.001

I am not the expert so I just can tell what I think from my point of view, base on the first result we can see if the widths are styled accurately, then the computed style of the buttons will be exactly 320 times 0.33 and 0.34, and the total computed width is 320 as well, which is the expected value we need.

And the second result shows that calc() does divide 320 by 3, then got 106.667 for each button. If we sum the three 106.667 and the width should become 320.001, which should cause the ui corrupted but didn't happen, I believe gecko has done some tricks to avoid this or 320.001 cannot be fitted into 320.

I think both approaches are acceptable and I am fine with 1 or 2, that's why I already gave r+ on your patch ^^. And base on the results I would say yes, it would be safer to use 33% 34% 33% instead of calc(100% / 3), the widths should be the expected values when the screen width is a multiple of 3.

Comment 17

[Jim Porter (:squib)]

Ok, I think I'll use 33% 34% 33% for this patch.

But in music2, let's just use CSS flexbox for everything. :)

Comment 18

[Preeti Raghunath(:Preeti)]

Jim

When can you have a patch ready for the same?

Comment 19

[Jim Porter (:squib)]

Landed: https://github.com/mozilla-b2g/gaia/pull/14793

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/gaia/commit/6c7ddcc1bba97fcc2864593a602c0c65aca1ff5c

Comment 21

[Inder]

(In reply to Michael Vines [:m1] [:evilmachines] from comment #14)
> Hey Inder, can you verify that the patch fixes the issue here?

Yes, don't see the issue with the patch. Thanks Jim for fixing!

Comment 22

[Inder]

Please uplift to v1.3.

Comment 23

[Jim Porter (:squib)]

Does this issue actually occur on an end-user device that will get 1.3? If not, I don't think we should uplift this. In any case, I think we have dedicated uplifters who know more about the uplifting process than I do...

Comment 24

[Jason Smith [:jsmith]]

(In reply to Jim Porter (:squib) from comment #23)
> Does this issue actually occur on an end-user device that will get 1.3? If
> not, I don't think we should uplift this. In any case, I think we have
> dedicated uplifters who know more about the uplifting process than I do...

To my understanding, this was reproduced on 1.3 originally, so we'll need to take this on the 1.3 branch.

Comment 25

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

Uplifted 6c7ddcc1bba97fcc2864593a602c0c65aca1ff5c to:
v1.3: 6a2c7dcbedc5dc73bbf4de3d531bc55cb1fe49fb