Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correction to Album listing (Default Skin) Track favourites link #1007

Merged
merged 2 commits into from Mar 10, 2024

Conversation

darrell-k
Copy link
Contributor

See https://forums.slimdevices.com/forum/user-forums/logitech-media-server/1681581-track-favourites-in-default-skin

The problem is that $songinfo (in Slim/Web/XMLBrowser.pm) always contains the album header info in the album view, so it should not be used to create a track favourite. I'm looking at $item instead, if it's a track (type==audio).

I've tried to be as light-touch as possible in this change, but:

  1. Has it really been wrong effectively for ever?
  2. Is this the best way of addressing the problem?

See https://forums.slimdevices.com/forum/user-forums/logitech-media-server/1681581-track-favourites-in-default-skin

The problem is that $songinfo (in Slim/Web/XMLBrowser.pm) always contains the album header info in the album view, so it should not be used to create a track favourite.
I'm looking at $item instead, if it's a track (type==audio).

I've tried to be as light-touch as possible in this change, but:
1. Has it really been wrong effectively for ever?
2. Is this the best way of addressing the problem?
@@ -1795,6 +1795,8 @@ sub _tracks {
delete $_->{'artwork_track_id'};
$_->{'playall'} = 1;
}

$_->{'favorites_url'} = $_->{'url'};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just use the item.url in the template? Would save one more line change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. I added that before full investigation, just in case its existence solved the problem. I'll remove it.

@michaelherger michaelherger merged commit 9abd585 into LMS-Community:public/8.5 Mar 10, 2024
@michaelherger
Copy link
Member

Thanks!

darrell-k pushed a commit to darrell-k/slimserver that referenced this pull request Mar 19, 2024
michaelherger pushed a commit that referenced this pull request Mar 20, 2024
…er to get the valid icon from the URL we (unfortunately) require a client object at this point. But in most cases we do have that, we only need to pass it around from within `Slim::Web::XMLBrowser`.

Don't use the `favorite_url` on songinfo in the web UI, as that would refer to the album rather than the track. This is a slightly different approach at fixing #1007.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants