[Merge] lp:~phablet-team/qtubuntu-media/fix-1449790 into lp:qtubuntu-media

[Alfonso Sanchez-Beato]

Review: Needs Fixing

My main concern with this change is whether we handle properly UTF-8 or not. The toLocal8Bit() suggests we might be breaking support for non-ascii file names (although I am not sure if we supported that before).

I think it is a good opportunity to add tests for UTF-8 (this MP adds infrastructure for that, like generate_test_files.sh, which is great) and make sure those files get played as expected.

Finally, I have an inline comment.

Diff comments:

> 
> === modified file 'src/aal/aalutility.cpp'
> --- src/aal/aalutility.cpp	2016-01-05 16:58:37 +0000
> +++ src/aal/aalutility.cpp	2016-02-15 16:25:06 +0000
> @@ -44,3 +44,16 @@
>  {
>      return unescape(media).toString().toStdString();
>  }
> +
> +std::string AalUtility::encode_uri(const QUrl &uri)
> +{
> +    std::string tmp {uri.toString().toLocal8Bit().toPercentEncoding("!$&'()*+,;=:/?[]@").constData()};
> +    // We want to remove the encoding for the '%' character otherwise it will cause media-hub
> +    // to not be able to find the filename. toPercentEncoding() will always encode the '%' character,
> +    // thus we can't simply add it to the exclude list.
> +    const std::string::size_type i = tmp.find("%25");
> +    if (i != std::string::npos)
> +        tmp.erase(i+1, 2);

I am not sure this is the right solution. If you have "%2521" it will translate to "!" instead of "%21", which would be the right string. I think media-hub should handle "%25" properly instead.

> +
> +    return tmp;
> +}


-- 
https://code.launchpad.net/~phablet-team/qtubuntu-media/fix-1449790/+merge/285814
Your team Ubuntu Phablet Team is subscribed to branch lp:qtubuntu-media.