1008586 - [Homescreen] Missing collections icons

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Everything.me, defect)

Description

[:gerard-majax]

On multiple devices, HIDPI (1.5x). Gaia is built with the correct env variables, all other icons are fine.

Logcat shows:
E/GeckoConsole( 7187): Content JS ERROR at app://homescreen.gaiamobile.org/gaia_build_defer_index.js:88 in retrieve: Got an exception when trying to load icon "app://homescreen.gaiamobile.org/collections/social/[e-mail] +" falling back to cached icon. Exception is: 
E/GeckoConsole( 7187): Content JS ERROR at app://homescreen.gaiamobile.org/gaia_build_defer_index.js:88 in retrieve: Got an exception when trying to load icon "app://homescreen.gaiamobile.org/collections/games/[e-mail] +" falling back to cached icon. Exception is: 
E/GeckoConsole( 7187): Content JS ERROR at app://homescreen.gaiamobile.org/gaia_build_defer_index.js:88 in retrieve: Got an exception when trying to load icon "app://homescreen.gaiamobile.org/collections/music/[e-mail] +" falling back to cached icon. Exception is: 
E/GeckoConsole( 7187): Content JS ERROR at app://homescreen.gaiamobile.org/gaia_build_defer_index.js:88 in retrieve: Got an exception when trying to load icon "app://homescreen.gaiamobile.org/collections/showbiz/[e-mail] +" falling back to cached icon. Exception is: 

I'm reproducing this constantly on master running on Flame, even after reset-gaia.

Comment 1

[Jason Smith [:jsmith]]

qawanted to check 1.4 here, although I'm sure this is likely a recent regression.

Comment 2

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

This issue did not occur on Flame 1.4 or Buri 1.4

Flame:
1.4 Environmental Variables:
Device: Flame 1.4 MOZ
BuildID: 20140512000204
Gaia: 17fb44880e95bc7ae363a609d811bf5a9a067b5b
Gecko: ec24f847e7c0
Version: 30.0
Firmware Version: v10F

Buri:
1.4 Environmental Variables:
Device: Buri 1.4 MOZ
BuildID: 20140512065808
Gaia: 2e86cd1cecfe24917cac8f47c3aa157ed7ef49d8
Gecko: 95bbb73c54d8
Version: 30.0
Firmware Version: v1.2-device.cfg

The collections icons appeared correctly on both devices.

Comment 3

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

Attachment: working screenshots on today's master

I can't seem to reproduce this issue on today's Flame master build. All icons in collections are loading correctly for me.

Attaching a screenshot combining Social, Games, Music, and Showbiz collection screenshots together.

Tested on:
Device: Flame MOZ
BuildID: 20140513040202
Gaia: de1c755411949b50ae395b42e124af215ed9b702
Gecko: 7f5a8526b55a
Version: 32.0a1
Firmware Version: v10F

Comment 4

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

Please read comment 3 above. We will need more information, perhaps screenshots, to reproduce this bug.

Comment 5

[:gerard-majax]

(In reply to Pi Wei Cheng from comment #4)
> Please read comment 3 above. We will need more information, perhaps
> screenshots, to reproduce this bug.

Which is unrelated, I'm speaking about the *homescreen* icons of collections. Not those inside the collections.

Comment 6

[:gerard-majax]

Attachment: 2014-05-14-06-57-22.png

Here is the screenshot to make it clear :)

Comment 7

[Cristian Rodriguez (:crdlc)]

are those icons "app://homescreen.gaiamobile.org/collections/xxx/[e-mail]" already available in the homescreen app? I think that they are not copied because I've unzipped the application.zip and those icons are not propagated. This is the issue. They are not loaded and the homescreen decides to paint icons by default

Comment 8

[:gerard-majax]

(In reply to Cristian Rodriguez (:crdlc) from comment #7)
> are those icons
> "app://homescreen.gaiamobile.org/collections/xxx/[e-mail]" already
> available in the homescreen app? I think that they are not copied because
> I've unzipped the application.zip and those icons are not propagated. This
> is the issue. They are not loaded and the homescreen decides to paint icons
> by default

I haven't checked the profile, but it would totally make sense. I see a lot of HIDPI-related icons issues. Do we have tests on this ?!

Comment 9

[:gerard-majax]

Yuren, I know you made changes to the buildsystem recently ; would this be a regression from this work?

Comment 10

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

homescreen app should access icon without the device pixel ratio suffix, but looks it try to access icons with that. George, could you help on this issue?

Comment 11

[Cristian Rodriguez (:crdlc)]

OK, I can do it in manifests:

https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/collections/funny/manifest.collection

from:

"icons": {
    "60": "/collections/funny/icon.png",
    "90": "/collections/funny/[e-mail]",
    "120": "/collections/funny/[e-mail]",
    "135": "/collections/funny/[e-mail]"
  }

to:

"icons": {
    "60": "/collections/funny/icon.png"
  }

Is the build in charge of copy as icon.png the correct size? Thanks

(In reply to Yuren [:yurenju] from comment #10)
> homescreen app should access icon without the device pixel ratio suffix, but
> looks it try to access icons with that. George, could you help on this issue?

Comment 12

[Cristian Rodriguez (:crdlc)]

Will do it

Comment 13

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

no it doesn't because build script only copies [e-mail] to icon.png, but it won't generate fields of manifest.

and George will help to figure out this is a regression of build system or not, so keep needinfo? to George.

Comment 14

[Cristian Rodriguez (:crdlc)]

so the manifest should be

"icons": {
  "60": "/collections/funny/icon.png"
}

there is only one icon and homescreen will paint it. In build time as you say, icon.png will be [e-mail] image, right?

Comment 15

[Cristian Rodriguez (:crdlc)]

Sorry I forgot the ni?

Comment 16

[(Inactive after June) George Duan [:gduan] [:喬智]]

Build script will filter icons by @(num)x.png based on GAIA_DEV_PIXELS_PER_PX to build_stage then to profile. So, there would be only one image left and renamed as icon.png.

If we want to remain below logic,
https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/js/grid.js#L1394
I suggest we can rename icon@(num)x.png and modify manifest.collection of each collection.

Hi Cris,
does that make sense to you? Or other suggestion?

Comment 17

[(Inactive after June) George Duan [:gduan] [:喬智]]

forgot to ni.

Comment 18

[Cristian Rodriguez (:crdlc)]

Current manifest:

"icons": {
  "60": "/collections/funny/icon.png",
  "90": "/collections/funny/[e-mail]",
  "120": "/collections/funny/[e-mail]",
  "135": "/collections/funny/[e-mail]"
}

We can do:

"icons": {
  "60": "/collections/funny/icon.png"
}

or:

"icons": {
  "60": "/collections/funny/icon.png",
  "90": "/collections/funny/90/icon.png",
  "120": "/collections/funny/120/icon.png",
  "135": "/collections/funny/135/icon.png"
}

What do you prefer?

Comment 19

[Cristian Rodriguez (:crdlc)]

I think that it is simpler to write the manifest like this [1] and the build script copy the correct icon

[1] "icons": {
  "60": "/collections/funny/icon.png"
}

Comment 20

[(Inactive after June) George Duan [:gduan] [:喬智]]

It looks a little weird. If the device pixel is actually 1.5 and we should only remain "90" one, 
then the "60" key is kinda confused.

Why not just make it as homescreen/style/icons/homeScreen_(num).png?

Comment 21

[Cristian Rodriguez (:crdlc)]

ok will do so, 10x

Comment 22

[Cristian Rodriguez (:crdlc)]

Attachment: Patch v1

Thanks for reviewing this mate

Comment 23

[(Inactive after June) George Duan [:gduan] [:喬智]]

Comment on attachment 8422358 [details]
Patch v1

r=gduan. LGTM.


In comment 18, choosing [2] will also not affect tablet's icon (see below).
https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/js/grid.js#L10

Comment 24

[Cristian Rodriguez (:crdlc)]

Merged in master:

https://github.com/mozilla-b2g/gaia/commit/5fd5c952cdc6f89613ccf6fa0f7f39fee9898bb9

Comment 25

[Cristian Rodriguez (:crdlc)]

Clearing 2.0? because it is already landed on master

Comment 26

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

(In reply to Alexandre LISSY :gerard-majax from comment #6)
> Created attachment 8422266 [details]
> 2014-05-14-06-57-22.png
> 
> Here is the screenshot to make it clear :)

Thanks for the screenshot. I wasn't seeing this issue either that's why I went into each collection to check. Looks like this issue is fixed so I won't need repro steps anymore.

Comment 27

[Jason Smith [:jsmith]]

Is this needed on 1.4? We're seeing the same bug here in 1.4 over in bug 1011612.

Comment 28

[Cristian Rodriguez (:crdlc)]

Hi Jason, I think that you're right! The problem seems the same. Thanks

(In reply to Jason Smith [:jsmith] from comment #27)
> Is this needed on 1.4? We're seeing the same bug here in 1.4 over in bug
> 1011612.

Comment 30

[Jason Smith [:jsmith]]

Root cause is due to bug 1010516.

Comment 31

[Brogan Zumwalt [Inactive]]

Adding smoketest keyword as this is a dupe of bug 1011612 which is a 1.4 smoketest blocker

Comment 32

[Brogan Zumwalt [Inactive]]

Comment 31 should say "bug 1011612 is a dupe of this bug" instead of the other way around.

Comment 33

[Ryan VanderMeulen [:RyanVM]]

v1.4: https://github.com/mozilla-b2g/gaia/commit/ce6c3d8fbd123dc65e8d2dd1f90403e24fa36dfe

Comment 34

[Josh Schmitt [Joshs]]

Bug does not repro in latest 1.4 Open_C build. All icons appear correctly with the correct image.

1.4 Environmental Variables:
Device: Open_C 1.4
BuildID: 20140521000202
Gaia: 93623f6435849cc9f54d9996e8e64828ac9091d1
Gecko: 12fe2b67a099
Version: 30.0
Firmware Version: P821A10V1.0.0B06_LOG_DL