Bug 4015 - Web setup corrupts screensaver settings in player UI
: Web setup corrupts screensaver settings in player UI
Status: CLOSED FIXED
Product: Logitech Media Server
Classification: Unclassified
Component: Web Interface
: 6.5b1
: PC All
: P2 normal (vote)
: ---
Assigned To: Chris Owens
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-08-29 03:01 UTC by Adrian Smith
Modified: 2008-12-18 11:12 UTC (History)
2 users (show)

See Also:
Category: ---


Attachments
return and use has for savers (2.51 KB, patch)
2006-08-29 10:48 UTC, KDF
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adrian Smith 2006-08-29 03:01:26 UTC
If the screensaver settings in the player UI are accessed after the player settings have been accessed the options are corrupted.

This is due to Web Setup corrupting the hash returned by Slim::Buttons::Common::hash_of_savers - it removes the underscores in the string names.  Simple fix is for Slim::Buttons::Common::hash_of_savers to return a copy of the hash rather than the hash itself, however can the web code be made to avoid modifying the hash?
Comment 1 KDF 2006-08-29 09:42:57 UTC
are you certain it's the web setup?  There is nowhere that seems to be making any changes to hash_of_savers.
Comment 2 Adrian Smith 2006-08-29 09:45:31 UTC
Yep - I put a Data::Dumper statement in Slim::Buttons::Common::hash_of_savers to display the current contents of the saver hash.  If you only use the player UI it is OK, but if you use the web it gets corrupted..
Comment 3 KDF 2006-08-29 09:57:57 UTC
ick.  could mean setup is hacking every hash used in validate or otions
Comment 4 KDF 2006-08-29 10:08:44 UTC
oh ok, I see.  It's the localised strings. clearly an options thing.  at least that narrows it down :)
Comment 5 KDF 2006-08-29 10:24:12 UTC
The problem comes from _sortOptionArray, which resolves tokens into strings for sorting the options list.  The caller is Options_HTTP.  The savers appears to be the only place that is returning a hashref.  Best fix at this point would be to return a hash, not a ref.  Future plans include an entire rework of the setup stuff.
Comment 6 KDF 2006-08-29 10:48:13 UTC
Created attachment 1480 [details]
return and use has for savers

this should work.
Comment 7 Adrian Smith 2006-08-29 12:08:19 UTC
Afraid that crashes if you try to set a new saver via the web interface:

Can't use string ("6/16") as a HASH ref while "strict refs" in use at /usr/local/slimserver/Slim/Utils/Validate.pm line 300.

Seeing as this only seems to impact the screensaver settings, how about my simple fix - make a copy of the hash before letting the setup code access to it:

--- trunk/server/Slim/Buttons/Common.pm 2006-08-29 18:17:43.000000000 +0100
+++ Slim/Buttons/Common.pm      2006-08-29 20:05:18.000000000 +0100
@@ -172,7 +172,10 @@
 =cut
 
 sub hash_of_savers {
-       return \%savers;
+
+       my %saversCopy = %savers;
+
+       return \%saversCopy;
 }
Comment 8 KDF 2006-08-29 12:17:06 UTC
if it works, I'm ok with it.  the whole thing needs to be ripped apart later so consistency really isn't a sticking point now.
Comment 9 Adrian Smith 2006-08-29 12:39:03 UTC
Added my fix to 9252, will merge to 6.5 too.

I'll close this, but it needs looking at again as part of any setup rework.
Comment 10 Chris Owens 2006-09-04 11:18:00 UTC
Dan would you like me to reopen this against version 7 for the setup rework Triode refers to?
Comment 11 Dan Sully 2006-09-04 11:27:13 UTC
Sure
Comment 12 KDF 2007-01-10 00:25:08 UTC
adrian, are we in the clear on this one yet?
Comment 13 Adrian Smith 2007-01-10 11:21:54 UTC
Setup rework in 7.0 covers this.
Comment 14 Chris Owens 2008-03-07 09:03:47 UTC
This bug is being closed since it was resolved for a version which is now released!  Please download the new version of SqueezeCenter (formerly SlimServer) at http://www.slimdevices.com/su_downloads.html

If you are still seeing this bug, please re-open it and we will consider it for a future release.