1022474 - Cannot add an e.me app to the homescreen via wrapper

Status: VERIFIED FIXED

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

Description

[Jason Smith [:jsmith]]

Build

* Device: Flame
* Version: 2.0 - b2g-inbound
* Build ID: 20140608173002
* Gaia: 12af93123c5db55212d51fe235d39f21209a1eaa
* Gecko: 7f3f35b511f8

STR

1. Open rocketbar & search for google
2. Select the Google icon
3. Open the wrapper

Expected

There should be a star icon allowing the user to add the e.me app to the homescreen.

Actual

There is no star icon there to allow the user to add the e.me app to the homescreen.

Comment 1

[Jason Smith [:jsmith]]

Fails smoketest - https://moztrap.mozilla.org/manage/case/5856/

Comment 2

[Cristian Rodriguez (:crdlc)]

Attachment: Github pull request

Thanks mate

Comment 3

[Kevin Grandon :kgrandon]

Comment on attachment 8436827 [details]
Github pull request

I asked some questions on github. Would like to see them addressed before giving an R+. Thanks!

Comment 4

[Dale Harvey (:daleharvey)]

Comment on attachment 8436827 [details]
Github pull request

Agreed that this doesnt seem like a feature we should be adding to the grid, we can add this unconditionally, and in future we wont need to do this at all as the bookmark button will be available all the time.

Comment 5

[Cristian Rodriguez (:crdlc)]

I am not agree. This feature is not needed for homescreen app. Bookmarks on homescreen have never show the star icon because they are already installed then it is not unconditional

(In reply to Dale Harvey (:daleharvey) from comment #4)
> Comment on attachment 8436827 [details]
> Github pull request
> 
> Agreed that this doesnt seem like a feature we should be adding to the grid,
> we can add this unconditionally, and in future we wont need to do this at
> all as the bookmark button will be available all the time.

Comment 6

[Cristian Rodriguez (:crdlc)]

I have answered questions

Comment 7

[Kevin Grandon :kgrandon]

I guess the concern is that the behavior in 2.1+ will change, but I suppose we should go with the easiest solution for 2.0 for now. I still feel weird about adding this as an attribute of the grid. I think that we should probably change the detail objects of bookmarks to allow for something special, or instantiate it a different way. I know it's a pain, but would it be difficult to explore doing something like the following:

var features = {search: true};
new Bookmark(detail, features);

Comment 8

[Cristian Rodriguez (:crdlc)]

Sincerely I don't know how you want to know if we have to include search feature or not. I decided to include this as an attribute instead of checking the origin (origin === homescreen). What do you suggest to know it if I cannot add an attribute? Thanks

Comment 9

[Kevin Grandon :kgrandon]

Sorry, to be more clear my suggestion is to see if there is some way that we can do it from the application side, when creating a grid object. This could potentially be part of the 'detail' object, or a new configuration.

E.g., you could update this: https://github.com/mozilla-b2g/gaia/blob/master/apps/search/js/providers/webresults.js#L54

To have something like: 
var features = {search: true};
new Bookmark(detail, features);

Cristian - is that any more clear?

Comment 10

[Cristian Rodriguez (:crdlc)]

yes

Comment 11

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8436827 [details]
Github pull request

thanks mates

Comment 12

[Kevin Grandon :kgrandon]

Comment on attachment 8436827 [details]
Github pull request

Looks good to me, thanks!

Comment 13

[Kevin Grandon :kgrandon]

Landed: https://github.com/mozilla-b2g/gaia/commit/ea4f0a93092daf8269b0b4410f6cd2d658c006e3

Comment 14

[Kevin Grandon :kgrandon]

Uplifted: https://github.com/mozilla-b2g/gaia/commit/f3e0fdd62c546e4086f0c52c0f0062c9ae7cb679

Comment 15

[Brogan Zumwalt [Inactive]]

Verified issue is fixed in 2.0 Flame

Result: Wrapper for e.me app shows favorite star

Environmental Variables:
Device: Flame 2.0
Build ID: 20140612000201
Gaia: 2bb66630315299ca947e40fcec23c9f7ea012111
Gecko: 670d69879f0e
Version: 32.0a2 (2.0) 
Firmware Version: v10G-2

Comment 16

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Verified 2.0: 

Gaia      23e06c3624309db22ad9cb736d89700768b42b36
Gecko     https://hg.mozilla.org/releases/mozilla-aurora/rev/164b61458ca5
BuildID   20140619000200
Version   32.0a2
ro.build.version.incremental=108
ro.build.date=Tue Jun 10 19:40:40 CST 2014
Flame

Comment 17

[Martin Shuman [:Marty]]

Verified fixed in Flame 2.1.
Issue no longer occurs.

Environmental Variables:
Device: Flame 2.1
BuildID: 20141021001201
Gaia: e458f5804c0851eb4e93c9eb143fe044988cecda
Gecko: ee86921a986f
Gonk: 05aa7b98d3f891b334031dc710d48d0d6b82ec1d
Version: 34.0 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0