1028664 - Collection icon rounds all app icons, but should only round E.me icons

Status: VERIFIED FIXED

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

Description

[Ran Ben Aharon [:ranbena] EverythingMe]

STR:
1. Drop a square shaped app into any Smart Collection.
2. Click on the SC and drag the newly added app to the first (left most) spot.
3. Return to homescreen.
4. Observer the newly added app centered in the SC icon.

Expected: The icon is square shaped
Actual: The icon is rounded.

Comment 1

[Ran Ben Aharon [:ranbena] EverythingMe]

Attachment: screenshot.png

In the screenshot you can see the "info" app being displayed rounded within The Social SC icon.

Comment 2

[Amir Nissim (:amirn)]

Attachment: Pull Request

Considering the alternatives, this patch applies the most simple solution for the bug.

Comment 3

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8455258 [details] [review]
Pull Request

It makes sense and LGTM, r+

Comment 4

[Kevin Grandon :kgrandon]

Comment on attachment 8455258 [details] [review]
Pull Request

I took a look at the patch and it looks fine to me. Also I think Cristians review will suffice here. Thanks!

Comment 5

[Amir Nissim (:amirn)]

landed: https://github.com/mozilla-b2g/gaia/commit/7eb98b5

https://tbpl.mozilla.org/?rev=2af842a3d63513fad8e2a18ee93289e8527eaaa9&tree=Gaia-Try

Comment 6

[Amir Nissim (:amirn)]

Comment on attachment 8455258 [details] [review]
Pull Request

[Approval Request Comment]

This is a dependency required for landing bug 1048005.

[Bug caused by] (feature/regressing bug #): regression
[User impact] if declined: Collection icons will display rounded app icons for all apps
[Testing completed]: yes
[Risk to taking this patch] (and alternatives if risky): very low
[String changes made]: only round icons for E.me web results

Comment 7

[bhavana bajaj [:bajaj]]

Comment on attachment 8455258 [details] [review]
Pull Request

BLocking on this given this dependency is required for landing bug 1048005 and clearing the approval here.

Comment 8

[Amir Nissim (:amirn)]

v2.0 https://github.com/mozilla-b2g/gaia/commit/6d7f264e9c1467c6f99c0f04165c73c8fd693ab9

Comment 9

[Elie]

Attachment: VIDEO0101.mp4

This issue has been successfully verified on Flame 2.0:
Gaia-Rev        8d1e868864c8a8f1e037685f0656d1da70d08c06
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/c756bd8bf3c3
Build-ID        20141201000201
Version         32.0
Device-Name     flame
FW-Release      4.4.2


This issue has been successfully verified on Flame 2.1:
Gaia-Rev        ccb49abe412c978a4045f0c75abff534372716c4
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/18fb67530b22
Build-ID        20141201001201
Version         34.0
Device-Name     flame
FW-Release      4.4.2