Closed Bug 1008586 Opened 6 years ago Closed 6 years ago

[Homescreen] Missing collections icons

Categories

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

x86_64
Linux
defect
Not set

Tracking

(blocking-b2g:1.4+, b2g-v1.4 fixed, b2g-v2.0 fixed)

VERIFIED FIXED
2.0 S2 (23may)
blocking-b2g 1.4+
Tracking Status
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: gerard-majax, Assigned: crdlc)

References

Details

(Keywords: regression, smoketest, Whiteboard: [systemsfe])

Attachments

(3 files)

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/icon@1.5x.png +" 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/icon@1.5x.png +" 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/icon@1.5x.png +" 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/icon@1.5x.png +" falling back to cached icon. Exception is: 

I'm reproducing this constantly on master running on Flame, even after reset-gaia.
blocking-b2g: --- → 2.0?
Summary: [Everything.me] Missing collections icons → [Homescreen] Missing collections icons
qawanted to check 1.4 here, although I'm sure this is likely a recent regression.
Keywords: qawanted
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.
Keywords: qawanted
Whiteboard: [systemsfe]
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
Please read comment 3 above. We will need more information, perhaps screenshots, to reproduce this bug.
Flags: needinfo?(lissyx+mozillians)
(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.
Flags: needinfo?(lissyx+mozillians)
Attached image 2014-05-14-06-57-22.png
Here is the screenshot to make it clear :)
Flags: needinfo?(pcheng)
are those icons "app://homescreen.gaiamobile.org/collections/xxx/icon@1.5x.png" 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
(In reply to Cristian Rodriguez (:crdlc) from comment #7)
> are those icons
> "app://homescreen.gaiamobile.org/collections/xxx/icon@1.5x.png" 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 ?!
Yuren, I know you made changes to the buildsystem recently ; would this be a regression from this work?
Flags: needinfo?(yurenju.mozilla)
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?
Flags: needinfo?(yurenju.mozilla) → needinfo?(gduan)
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/icon@1.5x.png",
    "120": "/collections/funny/icon@2x.png",
    "135": "/collections/funny/icon@2.25x.png"
  }

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?
Flags: needinfo?(yurenju.mozilla)
Will do it
Assignee: nobody → crdlc
Status: NEW → ASSIGNED
Flags: needinfo?(gduan)
no it doesn't because build script only copies icon@1.5x.png 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.
Flags: needinfo?(yurenju.mozilla) → needinfo?(gduan)
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 icon@1.5.png image, right?
Sorry I forgot the ni?
Flags: needinfo?(yurenju.mozilla)
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?
Flags: needinfo?(yurenju.mozilla)
Flags: needinfo?(gduan)
forgot to ni.
Flags: needinfo?(crdlc)
Current manifest:

"icons": {
  "60": "/collections/funny/icon.png",
  "90": "/collections/funny/icon@1.5x.png",
  "120": "/collections/funny/icon@2x.png",
  "135": "/collections/funny/icon@2.25x.png"
}

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?
Flags: needinfo?(crdlc) → needinfo?(gduan)
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"
}
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?
Flags: needinfo?(gduan)
ok will do so, 10x
Attached file Patch v1
Thanks for reviewing this mate
Attachment #8422358 - Flags: review?(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
Attachment #8422358 - Flags: review?(gduan) → review+
Merged in master:

https://github.com/mozilla-b2g/gaia/commit/5fd5c952cdc6f89613ccf6fa0f7f39fee9898bb9
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Clearing 2.0? because it is already landed on master
blocking-b2g: 2.0? → ---
(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.
Flags: needinfo?(pcheng)
Target Milestone: --- → 2.0 S2 (23may)
Is this needed on 1.4? We're seeing the same bug here in 1.4 over in bug 1011612.
Flags: needinfo?(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.
Flags: needinfo?(crdlc)
Duplicate of this bug: 1011612
blocking-b2g: --- → 1.4+
Root cause is due to bug 1010516.
Blocks: 1010516
Adding smoketest keyword as this is a dupe of bug 1011612 which is a 1.4 smoketest blocker
Keywords: smoketest
Comment 31 should say "bug 1011612 is a dupe of this bug" instead of the other way around.
Keywords: smoketest
Keywords: smoketest
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
Status: RESOLVED → VERIFIED
Duplicate of this bug: 1017532
You need to log in before you can comment on or make changes to this bug.