1211932 - [Music][NGA] a % in the song filename wreak havoc

Status: RESOLVED FIXED

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

Description

[Hubert Figuiere [:hub]]

a % in the song filename wreak havoc

I get a lot of "E/Music   (15846): [JavaScript Error: "URIError: malformed URI sequence" {file: "app://music.gaiamobile.org/js/view.js" line: 118}]"

The URL is something like
/api/artists/info//sdcard1/Music/Ogg/Sonic%20Youth/Dirty/01%20-%20100%.ogg 

or

/api/artwork/url/thumbnail//sdcard1/Music/Ogg/Sonic Youth/Dirty/01 - 100%.ogg 

We should escape '%' correctly when building the URL with file names.

Comment 1

[Justin D'Arcangelo [:justindarc]]

I thought we were doing this, but must be missing it somewhere.


BTW, great song/album/band :-)

Comment 2

[Hubert Figuiere [:hub]]

Looking at it

Comment 3

[GitHub Autolander Bot]

Attachment: [gaia] hfiguiere:bug1211932-percent > mozilla-b2g:master

Comment 4

[Hubert Figuiere [:hub]]

Comment on attachment 8670607 [details] [review]
[gaia] hfiguiere:bug1211932-percent > mozilla-b2g:master

Wilson,

In order to fix the problem I had to patch poplar.js. Any thought? Shall we ensure the patch is upstream? Is there a more elegant solution? I assume that a "href" attribute has to be uri encoded.

Comment 5

[Hubert Figuiere [:hub]]

Comment on attachment 8670607 [details] [review]
[gaia] hfiguiere:bug1211932-percent > mozilla-b2g:master

cancelling feedback for Wilson: I no longer touch poplar.js

Justin, your thoughts? the code path for poplar is so much faster now.

Comment 6

[Hubert Figuiere [:hub]]

I should write a test for this one.

Comment 7

[Justin D'Arcangelo [:justindarc]]

Comment on attachment 8670607 [details] [review]
[gaia] hfiguiere:bug1211932-percent > mozilla-b2g:master

I agree with the approach. Please see my comments on the PR though regarding using `map()` to create new objects instead of using `forEach()` to alter the original objects. Also, we can contain most of the calls to `encodeURIComponent` by putting it inside the `fetch()` method in the base `View` class (in "js/view.js").

Comment 8

[Wilson Page [:wilsonpage]]

hub: If you can post *very* detailed STR I can try to reproduce :)

Comment 9

[Hubert Figuiere [:hub]]

This one is being fixed as we speak.

Comment 10

[Hubert Figuiere [:hub]]

It doesn't touch gaia-fast-list, btw.

Comment 11

[Hubert Figuiere [:hub]]

Comment on attachment 8670607 [details] [review]
[gaia] hfiguiere:bug1211932-percent > mozilla-b2g:master

I think I should have it all cornered now.

I'm curious of the memory usage and perf gain.

Comment 12

[Justin D'Arcangelo [:justindarc]]

Comment on attachment 8670607 [details] [review]
[gaia] hfiguiere:bug1211932-percent > mozilla-b2g:master

Looks good. You can go ahead and clean up the "/player" URLs as mentioned in the comments before landing (sorry I didn't realize this sooner). Thanks!

Comment 13

[Hubert Figuiere [:hub]]

Apparently I have an Orange. *sigh*

Comment 14

[Hubert Figuiere [:hub]]

Comment on attachment 8670607 [details] [review]
[gaia] hfiguiere:bug1211932-percent > mozilla-b2g:master

There was an error in the code that caused a failure that I missed on the integration test. I added a commit to the PR to fix it. If you don't mind reviewing these two one-liners...

Thanks,

Comment 15

[Hubert Figuiere [:hub]]

Merged
https://github.com/mozilla-b2g/gaia/commit/74b0d4b17f39d238a7997800bd9363d3c60f20c3