Bug 4083 - player_chooser_list can be problematic with long player names
: player_chooser_list can be problematic with long player names
Status: RESOLVED WONTFIX
Product: Logitech Media Server
Classification: Unclassified
Component: Skins
: 6.5b1
: PC All
: P2 minor (vote)
: Future
Assigned To: Ben Klaas
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-09-11 08:34 UTC by Ben Klaas
Modified: 2011-01-13 08:26 UTC (History)
1 user (show)

See Also:
Category: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ben Klaas 2006-09-11 08:34:24 UTC
The server-provided variable, player_chooser_list, can be problematic when the player name is long. What happens, at least in some skins (Nokia770, for one), is that when the select menu exceeds the width of the containing <div> tag, the select menu renders as a "blip". This especially happens when several players are synced with each other, thus creating a very long string, as Mike Gilpin describes on the Nokia770 thread:

http://forums.slimdevices.com/showpost.php?p=134249&postcount=560

I have alleviated this issue somewhat by dropping the select menu into it's own div tag that spans the entire page width, but I think a more complete fix would truncate the player string that is dropped into the option tag in the select menu. Since player_chooser_list is controlled by the Slim code itself, and this change would affect all skins, I thought I'd throw this out as a bug and see what the opinion was before I checked anything in...

this is the fix I propose--

Index: Slim/Web/Pages.pm
===================================================================
--- Slim/Web/Pages.pm   (revision 9587)
+++ Slim/Web/Pages.pm   (working copy)
@@ -187,6 +187,7 @@
                                $clientlist{$eachclient->id()} .= " (".string('SYNCHRONIZED_WITH')." ".
                                        Slim::Player::Sync::syncwith($eachclient).")";
                        }
+                       $clientlist{$eachclient->id()} = substr($clientlist{$eachclient->id()}, 0, 50);
                }

                $params->{'player_chooser_list'} = $class->options($client->id(), \%clientlist, $params->{'skinOverride'});
Comment 1 Blackketter Dean 2006-09-16 19:30:43 UTC
going to punt this for consideration in 7.0.  

One more thought, Ben: can you do this processing in your skin rather than dropping the name (which the user presumably likes) for all skins?
Comment 2 Ben Klaas 2006-09-18 07:37:16 UTC
>One more thought, Ben: can you do this processing in your skin rather than
>dropping the name (which the user presumably likes) for all skins?

I'd actually love to do that, but the entire select list is given with the template var [% player_chooser_list %]. If I had finer grain control of those list items I'd use TT's truncate() filter. But since I don't, I can't.

FWIW, the user that reported this issue is quite happy with the partial fix I put in at the skin level.
Comment 3 Michael Herger 2007-10-18 03:20:54 UTC
You can create the list yourself in the skin, if you prefer. I'm doing this in Handheld (see pageheader.html):

[%- USE Clients; players = Clients.get("id") -%]
[% IF players.size > 1 %]
	<form method="get" action="[% webroot %]status.html">
		<select name="player" onchange="switchPlayer(this);">
		[% FOREACH playerobj IN players -%]
			<option value="[% playerobj.id %]" [% IF player == playerobj.id %]SELECTED[% END %]>[% playerobj.name %]</option>
		[% END %]
		</select>
	</form>
[% END %]

Though this won't display the synchronization status.

The new Default skin is handling the list independantly, doesn't use neither method. Instead it builds it dynamically, based on the serverstatus CLI command (over JSON/RPC).
Comment 4 Michael Herger 2008-01-08 10:35:59 UTC
Ben - can I close this bug?
Comment 5 Ben Klaas 2008-01-08 11:11:45 UTC
let's keep it on my pile so I can address the issue in Nokia770/Touch

pushing target to future, severity to minor, and assigning to me.