1126709 - Should use the URL() object instead of string concatenation to resolve icon urls for Homescreen application.

Status: RESOLVED WONTFIX

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

Description

[Josh Cheng [:josh]]

According to https://bugzilla.mozilla.org/show_bug.cgi?id=1069810#c6. Should use the URL() object instead of doing string concatenation to resolve urls.

Comment 1

[Josh Cheng [:josh]]

Attachment: Pull Request for master

Hi Kevin,
Could you please review the patch? This is to resolve the issue per bug 1069810. Thanks!

Comment 2

[Kevin Grandon :kgrandon]

Comment on attachment 8555731 [details] [review]
Pull Request for master

Fabrice was looking for this so I'd like to defer to him here. I think it makes sense to me, though it looks like the arguments are backwards? Did you test it?

Comment 3

[[:fabrice] Fabrice Desré]

Comment on attachment 8555731 [details] [review]
Pull Request for master

Commented on github.
After this one, could you cleanup shared/js/url_helper.js to also use the URL object?

Comment 4

[Josh Cheng [:josh]]

Hi Fabrice,
Thanks for the feedback. I will handle this soon.

Comment 5

[Josh Cheng [:josh]]

Attachment: PR V2 include url_helper.js

Hi Fabrice, 
Could you please review again? Thanks!

Comment 6

[Josh Cheng [:josh]]

Attachment: PR for master

Hi Fabrice,
Thanks for the comment.
Could you please review again?

Comment 7

[[:fabrice] Fabrice Desré]

Comment on attachment 8557464 [details] [review]
PR for master

Thanks Josh, look good. Waiting for the tests results to be complete though (I retriggered Gu failures).

Comment 8

[[:fabrice] Fabrice Desré]

Josh, the Gij failure seem real too. Please check that before and ask for review again.

Comment 9

[Gregor Wagner [:gwagner]]

[Blocking Requested - why for this release]:

Comment 10

[Gregor Wagner [:gwagner]]

Since bug 1069810 is 2.0+

Comment 11

[Josh Cheng [:josh]]

Hi Greg, 
I have asked Kai-ZHen to backout bug 1069810 and wait until this fix. Currently Gij is broken with this fix and Evan Tseng is helping me for this. 
Thanks!

Comment 12

[Evan Tseng [:evanxd]]

Use the patch to run the Gij task[1] for debugging.

[1]: https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=4b122d7b1ca8509cbec67c671720d8650f23ba3b

Comment 13

[Josh Cheng [:josh]]

Comment on attachment 8557464 [details] [review]
PR for master

Hi Fabrice,

The test are all green without change on my patch. 
Could you please help to review again? Thanks!

https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=4baf604f247975e23ae8d1a3786fa9489ed0990e

Comment 14

[[:fabrice] Fabrice Desré]

Comment on attachment 8557464 [details] [review]
PR for master

thanks Josh!

Comment 15

[GitHub Autolander Bot]

Autolander could not locate a review from a user within the suggested reviewer list. Either the patch author or the reviewer should be in the suggested reviewer list.

Comment 16

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

land in master:
https://github.com/mozilla-b2g/gaia/commit/1966559e618575875b8cfebde3bf8165acb94be1

Comment 17

[KTucker [:KTucker][Inactive 3/4/2016]]

We need this backed out. It is causing a smoke test blocker. Please see bug 1159299.

Comment 18

[Kevin Grandon :kgrandon]

Backed out for causing bug 1159299.

https://github.com/mozilla-b2g/gaia/commit/cc498e53ff6ef200deebebb55ec26f1d72bd1d90

Comment 19

[Kevin Grandon :kgrandon]

Mass update: Resolve wontfix all issues with legacy homescreens.

As of 2.6 we have a new homescreen and having these issues open is confusing. All issues will block bug 1231115 so we can use that to re-visit any of these if needed.