escape ' in favorite titles

[philippe44]

Somebody with a higher paygrade please explain why the 'html' filter does not seem to escape the quote, but I spent a good amount of time when I was trying to fix the favorite thing because OF COURSE the album I was trying and its 1st track had a quote in the f*** name/title and that was (sort of) silently failing

[philippe44]

Found out ... in CPAN::Template::Filters.pm, the html filter does not replace quote

[philippe44]

ok, this time I think I nailed it.

@darrell-k, you were mostly right, the change had to be in xmlbrowser.html as we must always keep the songinfo which sometimes contains ... album info. But that "songinfo" (in Classic) is used when drilling an album, at the top right of the album icon, to display the fav, play... icons and that's what is used to add the album to favorites. But when adding a track to favorites, in the list below, we should NOT use that songinfo, obviously. We must let XMLBrowser.pm to drill down again and do the add.

Still, when we have drilled down an individual track (either clicking on it or using the M(ore) button, then we must use again the songinfo, we can't drill down as we have lost hierarchy context.

Last but not least, the random name that we saw was because we were using a non-existing (bread)crum attribute... So we absolutely can use that part of the code to create the proper favorite, in fact we should, whether it's database or plugin.

[michaelherger]

Is this PR replacing #1017? It would basically revert the previous change, but make sure tracks would not use the the album info to create the favs?

And yes, the single quote is a mess.

[philippe44]

Is this PR replacing #1017? It would basically revert the previous change, but make sure tracks would not use the the album info to create the favs?

Yes. I mean it addresses it

And yes, the single quote is a mess.

And Yes, cost me a few hours...

[michaelherger]

What kind of URL would that add to the favorites for a single track? I'm failing to read what the ELSE does there...

[philippe44]

I'm not sure what you mean by "what type of URL", but the actual result depends if you click on the favicon in the list of tracks or if you click on favicon after you opened the track details. It does not follow the same route in xmlbrowser.html.

I was also about to offer an addition to that PR that allows, when we take the 2nd route to add the icon which is missing today. See

slimserver/HTML/Default/xmlbrowser.html

Line 210 in 6c11c5b

[%- BLOCK allcontrol -%]

and

slimserver/Slim/Plugin/Favorites/Plugin.pm

Line 251 in 6c11c5b

sub toggleButtonHandler {

[michaelherger]

I obviously haven't tested this PR yet... but wouldn't a track item favourite automatically pull in the track's cover as defined in the database?

I'll play with this in a few hours.

[philippe44]

I obviously haven't tested this PR yet... but wouldn't a track item favourite automatically pull in the track's cover as defined in the database?

I'll play with this in a few hours.

Yes, except that this does not work for plugins where you want to add a favs this way but are not in the db. That's what I'm facing with Deezer

[philippe44]

@darrell-k should also give his opinion

[michaelherger]

I believe the following change should fix the icon for remote items added through the Default web UI (in addition to this PR):

diff --git a/Slim/Web/XMLBrowser.pm b/Slim/Web/XMLBrowser.pm
index 55943009c..3028bff85 100644
--- a/Slim/Web/XMLBrowser.pm
+++ b/Slim/Web/XMLBrowser.pm
@@ -540,7 +540,7 @@ sub handleFeed {
 			 && !($subFeed->{type} && $subFeed->{type} eq 'search')
 			 && !(ref $subFeed->{'url'}) )
 		) {
-			$subFeed->{'image'} ||= $subFeed->{'cover'} || $subFeed->{'icon'} || Slim::Player::ProtocolHandlers->iconForURL($subFeed->{'play'} || $subFeed->{'url'});
+			$subFeed->{'image'} ||= $subFeed->{'cover'} || $subFeed->{'icon'} || Slim::Player::ProtocolHandlers->iconForURL($subFeed->{'play'} || $subFeed->{'url'}, $client);
 			$subFeed->{'image'} = proxiedImage($subFeed->{'image'});
 
 			$stash->{'streaminfo'} = {
@@ -999,7 +999,7 @@ sub handleFeed {
 
 				my $type = $item->{'favorites_type'} || $item->{'type'} || 'link';
 				my $name = $item->{'favorites_title'} || $item->{'name'};
-				my $icon = $item->{'favorites_icon'} || $item->{'image'} || $item->{'icon'} || Slim::Player::ProtocolHandlers->iconForURL($furl);
+				my $icon = $item->{'favorites_icon'} || $item->{'image'} || $item->{'icon'} || Slim::Player::ProtocolHandlers->iconForURL($furl, $client);
 
 				if ( ($item->{'play'} && !$item->{'favorites_type'})
 				    || ($type eq 'playlist' && $furl =~ /^(file|db):/)

[darrell-k]

I think this is the way to go.

But, testing this I've noticed that adding an album to favorites from the list of albums is not working. But this also happens in the current 9.0 (with my previous PR #1007)

Am investigating...

[michaelherger]

I don't know why this PR isn't updated. I committed it as part of my suggested change. But this PR doesn't pick up the new baseline...

See acfae69

[darrell-k]

You committed to 8.5, but not 9.0.

[darrell-k]

I think this is the way to go.

But, testing this I've noticed that adding an album to favorites from the list of albums is not working. But this also happens in the current 9.0 (with my previous PR #1007)

Am investigating...

Something is screwed up in my environment: it works fine on my Android Phone LMS, but not on my laptop. Both running stock 8.5.

What do I have to do to definitively clear all caching (including javascript) in /var/lib/squeezeboxserver/cache and in Firefox?

[mherger]

You committed to 8.5, but not 9.0.

This PR is against 8.5, isn't it?

[mherger]

Looking good enough to me.

[michaelherger]

So I merged this, and it resulted in an empty commit. Something's wrong with GH today...

[darrell-k]

You committed to 8.5, but not 9.0.

This PR is against 8.5, isn't it?

Ah yes, mine was against 9.0. sorry for the confusion!