CardsHelper.getIconURIForApp returns bad url given an absolute, off-origin URL

RESOLVED FIXED in 2.1 S4 (12sep)

Status

Firefox OS
Gaia::System
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: sfoster, Assigned: sfoster)

Tracking

unspecified
2.1 S4 (12sep)
x86_64
Linux

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [systemsfe])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

3 years ago
e.me apps return a .icon url like "http://api.everything.me/partners/1.0/Icon/get?id=26&iconFormat=20&apiKey=...", which is getting badly resolved/concat-ed with the app origin to produce "http://wikipedia.flyapps.mehttp://api.everything.me/partners/1.0/Icon/get?id=26&iconFormat=20&apiKey=..."
(Assignee)

Comment 1

3 years ago
Created attachment 8485851 [details] [review]
Fix handling for absolute icon URLs used by e.me app windows

Rather than assuming all non-data URIs are relative paths, this change explicitly checks for leading '/' - which is the only expected and handled case in this block. 

The getOriginObject a) wasnt used outside this file, and b) was really just returning a URL-like hash with protocol etc. properties. So I got rid of it in favour of using URL.
Attachment #8485851 - Flags: review?(etienne)
Comment on attachment 8485851 [details] [review]
Fix handling for absolute icon URLs used by e.me app windows

lgtm :)
Attachment #8485851 - Flags: review?(etienne) → review+
(Assignee)

Comment 3

3 years ago
Merged to master: https://github.com/mozilla-b2g/gaia/commit/cebf9e694d4536fbc4012a1fe1f60d31adf8e628
Original commit: https://github.com/mozilla-b2g/gaia/commit/0294015fca59f4f8d08ae8c946c7a3fccce32310

..but I was looking at the wrong tbpl results. Will backout and try again shortly.
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Assignee)

Comment 4

3 years ago
Backed out: https://github.com/mozilla-b2g/gaia/commit/46e35fd6adc8f89d1b058ff84312cb38242c4658
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Updated

3 years ago
Blocks: 1061324
(Assignee)

Comment 5

3 years ago
Created attachment 8485971 [details] [review]
Fix handling of absolute icon urls in CardsHelper (redux)

Carrying Etienne's r+, this PR fixes unit tests by handling empty src urls which get passed in the tests.
Attachment #8485851 - Attachment is obsolete: true
Attachment #8485971 - Flags: review+
(Assignee)

Updated

3 years ago
Assignee: nobody → sfoster
Target Milestone: --- → 2.1 S4 (12sep)
You need to log in before you can comment on or make changes to this bug.