Bugzilla – Bug 3649
"Forget this player" forgets wrong player
Last modified: 2009-01-29 09:46:59 UTC
Running on a Nightly (based on SVN 7720) SVN upgraded to 8168 I had a SB2 and a Softsqueeze registered. The link to Player Settings (from the frontpage) have two player parameters (player and playerid). For the SB2 these were the same (the mac-address for the SB2), but for the SoftSqueeze only "player" points to the SoftSqueeze "mac-address". The playerid was pointing to the SB2 address. So when I clicked the Player Settings for the SoftSqueeze and then clicked "forget this player" the link contained the mac-address for the SB2 in the player parameter, with the result that my SB2 was forgotten.
This appears to be caused by the changePlayer() script. Changing the player from the right side plulldown appears to be changing both the player and playerid params. Will have a longer look at this later today
in HTML/EN/html/common.js change the first case rExp in switchPlayer() to: var rExp = /(player=(\w\w(:|%3A)){5}(\w\w))|(player=(\d{1,3}\.){3}\d{1,3})/gi; this will ensure that only the player param is changed, and player id is intact. Leaving the second case alone will allow the player switcher to change pages if the player pulldown is changed. Although, given that all player settings are accessible from home.html, this means that the player setup page is already not matching the player pulldown at all times. possibly not a problem since the user does know which player link was used. Changing the player from the pulldown, one should expect the left side to change to match. Chris, I can't commit this until this evening anyway, so please feel free to assign to the best person to review, or it can be assigned to me and I'll go ahead and merge it in later.
Dan can reassign it to Deane depending on how busy he is, etc. :)
looks like this problem could also affect 6.3. The same switch player script is in use there, same patch applies. applied to 6_3_x branch at change 8190 (in case 6.3.1 ends up coming along) applied to trunk at change 8191, along with changelog entry. please reopen if there are any problems.
Sorry, I can still get it to fail. I SVN upgraded to 8229 to test, and was still able to reproduce the error. Procedure: Only my SB 2 registered. Start album from Web UI Start SoftSqeeze from WebUI When I then clicked Home two players was registered. On the Softsqueeze UI, start another album. Now both players are playing so we can see if they loose connection to the server. The right side pane is now showing the SB2 but with the combobox shown. The two player settings link's parameter: SB2: player=SB2&playerid=SB2 Softsqueeze: player=SB2&playerid=SoftSqueeze Now I click the player settings for the Softsqueeze And the link to Forget player has these parameters Playerid=Softsqueeze&player=SB2 When I click the Forget player link I get to the frontpage with only the SoftSqueeze shown (which I just tried to remove) The right side shows the playlist for the Softsqueeze (and no Combobox), but with only the current song listed and not the whole album which I selected earlier, and since the player by default is on Repeat All it continues to play this song. I now stops the SoftSqueeze program. After some time I try to refresh the whole WebUI (pressing F5 in my browser), and then no players are listed. I don't know is this is an error or if SoftSqueezes should disapear when the player is un-powered (program not running) The SB2 stops playing after some time (When the buffer runs dry)
I think the command client->forget is taking the currently active player, instead of the playerid param. The dispatcher is set to call: Slim::Control::Commands::clientForgetCommand] with no args, so falls to the current player=, instead of playerid= Fred, any ideas?
Slim::Commands::clientForgetCommand acts on the client attribute of the request object. The client attribute of the request object is set to the value of the first param of Slim::Control::Request::executeRequest, named $client in Slim::Web::HTTP::executeURL. This last routine attempts to define $client using the 'player' param, or from a cookie, or finally defaults to random. If a request is set to require a client, as clientForgetCommand is (first array value 1 in second array of dispatcher table), the request will not fire if there is no client defined. So if the client was randomized when clientForgetCommand executes from a web call, it happened in Slim::Web::HTTP::executeURL, and not at any point after Slim::Control::Request::executeRequest. Hope this helps
thanks Fred. I understand WHY it is going wrong now, assuming executeURL really means processURL. However, I don't understand how a command that USED to work with the url in place now, no longer does. processURL simply passes on p2 as it always did, but now playerid is ignored. I can't seem to figure out how to get at the url params from the request, and I would have expected that anything sent via url should be connected to the CLI. Is that not what it is for? surely we can't possibly be suggesting that the right thing to do is to go further and further into diverging services within one package. If the param is available to the CLI, the fix is painfully easy...but I can't find the param even though processURL is sending the @p array to executeRequest. Is there a way that can get lost, or a way that I'm missing for extracing it?
Created attachment 1309 [details] send proper player param to the CLI The CLI forgetClient command takes only a client. We have to send the right player MAC in the player param in order to forget the correct client. This means taking the playerid param and sending it as the player for the purposes of this one url. The page is reloaded and the HTTP module handles picking a valid player since the player param would at that time be no longer valid (forgotten)
Subject: Re: "Forget this player" forgets wrong player Looks good to me.
I just added the changes to my server, and the Forget Player now forgets the correct player (running Default skin). But I discovered another behavior that may not be so lucky (It was also there before the changes). Scenario: One player registered. Click Player settings Click Forget player Player stops Cycle power on player so it re-registeres Since the web-interface is already on 'Home' there are no 'Home' link to "force" a refresh of the left side of the UI, so I click refresh in my browser to get everything refreshed, but the URL contains the forget link, so my player is forgotten again. So perhaps a redirection to '/' after the player was forgotten would be an idea, to clean up the URL in the browser.
committed to trunk at change 8267. redirect is easier said than done, so that still exists and leaving this open until someone else decides the right way to do that or that it isn't necessary.
Can the redirect happen with JS?
I tried to do a secondary location.replace, but it would always end up swallowing the first replace. I even tried putting the redirect as a timer, waiting 4 seconds before redirect and it still swalled the command url so that no player change took place. I felt that more than 4 seconds, and it would be a rather annoying forced refresh experience.
Alright. I'm ok with it as is. Moving to future, if someone comes up with an idea for the redirect/refresh.
If the problem is the forget is acted upon after a refresh, could we add a timestamp and only do the forget if the timestamp is "fresh" and if not redirect to "pure" home? If you click on forget way after the page was generated, you'll end up in home (with the player still there) and the natural reaction would be to try it again, and this time it will work. Just a thought.
Created attachment 1474 [details] Display one track behind but still playing
What the? I tried attaching this to another bug and ended up here. Please ignore.
Created attachment 1766 [details] different approach might work best using an onClick for the link instead of an http command query. We can send the command via an xmlHTTPRequest post and maybe a page refresh on callback. we could eventually extend this to make it generic enough for a number of comment requests that we want to hide from the browser refresh (bug 2184, for example)
fix committed to trunk at change 11194. This will not work on Handheld which doesn't support the use of javascript or XMLHttpRequests.
KDF states this was fixed. If anyone feels otherwise please let us know. :)
anyone using Handheld skin won't get a fix :) It's just not designed for use with devices that support the technology required.
This bug appears to have been fixed in the latest release! If you are still experiencing this problem, feel free to reopen the bug with your new comments and we'll have another look. Make sure to include the version number of the software you are seeing the error with.