902786 - e.me in landing page is not multi-dppx ready

Status: VERIFIED FIXED

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

Description

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

The patch in bug 838634 does not take dppx into account. 

https://github.com/mozilla-b2g/gaia/commit/60ca81600a080dae33058b0692ecaa213556c926#L7R228

This should be 'sans-serif', not a particular font-family name.

https://github.com/mozilla-b2g/gaia/blob/60ca81600a080dae33058b0692ecaa213556c926/apps/homescreen/everything.me/js/helpers/Utils.js#L27

It still calculate poor man's dppx.

https://github.com/mozilla-b2g/gaia/commit/60ca81600a080dae33058b0692ecaa213556c926#L22R99

The hack does not include helix device (device css height/width = 320x533).


I humbly wish that patch authors and reviewers understanding there there is a on-going effort of shipping an hd device on top of v1-train branch, before leo+ and landing a patch.

Comment 1

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

Ran, would you submit a follow up patch, for master and v1.1.0hd?

Vivien, would you review the follow-up patch?

Comment 2

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

It looks like the entire patch is broken on v1.1.0hd when GAIA_DEV_PIXELS_PER_PX is set to 1.5. We will need a follow-up fix in this bug too.

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

I would review whatever is needed and pops up before friday. Thanks for opening the bug.

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #0)
> The patch in bug 838634 does not take dppx into account. 
> 
> https://github.com/mozilla-b2g/gaia/commit/
> 60ca81600a080dae33058b0692ecaa213556c926#L7R228
> 
> This should be 'sans-serif', not a particular font-family name.
> 

So just changing |Feura Sans| to |sans serif| would do it? 
That's what I thought at first but I think Ran told me he had to use an explicit name for canvas. We will check that again.


> https://github.com/mozilla-b2g/gaia/blob/
> 60ca81600a080dae33058b0692ecaa213556c926/apps/homescreen/everything.me/js/
> helpers/Utils.js#L27
> 
> It still calculate poor man's dppx.
>

Does s/320/window.innerWidth is enough hre?
 
> https://github.com/mozilla-b2g/gaia/commit/
> 60ca81600a080dae33058b0692ecaa213556c926#L22R99
> 
> The hack does not include helix device (device css height/width = 320x533).
> 

Oh cool! I stole this hack from the browser app directly. Sounds like the browser app needs a patch then I will stole it again :)
 
> I humbly wish that patch authors and reviewers understanding there there is
> a on-going effort of shipping an hd device on top of v1-train branch, before
> leo+ and landing a patch.

Tell that to planning!

Comment 4

[Ran Ben Aharon [:ranbena] EverythingMe]

Attachment: Patch - redirect to github PR.html

Comment 5

[Ran Ben Aharon [:ranbena] EverythingMe]

Tim, it looks like the merge simply removed some important stuff from Utils.js that we put in to the hd branch a few weeks ago. So I just put it back and things seem fine.

I couldn't connect the dppx phone to wifi here so I could test the apps. How does it perform now?

Comment 6

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

Comment on attachment 787733 [details]
Patch - redirect to github PR.html

Thanks! I will test it and see if there is any follow-up issues.

Comment 7

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

v1.1.0hd: https://github.com/mozilla-b2g/gaia/commit/65bd444454bfef0be653272d277439c6ac75787b

Comment 8

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

I can verify the patch does fix E.me on hd branch. However we need the same fix (aligning hd with master) on master.

https://github.com/EverythingMe/gaia/blob/master/apps/homescreen/everything.me/js/helpers/Utils.js

Ran, would you submit a patch for that too? Thanks.

Comment 9

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

Ran, you need to patch the master branch too in order to close this bug.

Comment 10

[Ran Ben Aharon [:ranbena] EverythingMe]

Hey Tim, we've landed the fix in Bug 910338. Can you verify?

Comment 11

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

Right. We should mark this bug as unaffected to v1.2 since the UI changed a lot.

Thanks.

Comment 12

[Al Tsai [:atsai]]

[2013/10/21 Helix Testing]
Gaia:     c829a2042594b6c3a4899ee27979799a0f301534
Gecko:    http://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/f7c657f6d019
BuildID   20131015042201
Version   18.0