Bug 15001 - Save playlist plugin locks SB classic UI
: Save playlist plugin locks SB classic UI
Status: RESOLVED FIXED
Product: Logitech Media Server
Classification: Unclassified
Component: Plugins
: 7.4.1
: PC Windows Vista
: -- normal (vote)
: ---
Assigned To: KDF
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-10-31 14:20 UTC by rgrant
Modified: 2012-03-13 00:25 UTC (History)
2 users (show)

See Also:
Category: Bug


Attachments
back out after showBriefly (1.64 KB, patch)
2009-11-03 16:36 UTC, KDF
Details | Diff
Resolves use via extras menu (comment #12) (570 bytes, patch)
2012-01-15 11:41 UTC, Martin Williams
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description rgrant 2009-10-31 14:20:36 UTC
Using SQB3, server 7.4.1, Save Playlist plugin.

When I press and hold the play button from a track in the Now Playing area, and save the playlist, the SQB UI returns to the Now Playing playlist. Pressing right-arrow shows TrackInfo. But pressing left arrow from the Now Playing playlist shows "THE CURRENT PLAYLIST", and the SQB classic UI no longer responds to the remote. I'm using the Date and Time screen saver as well. When the problem occurs, the Date and Time screen saver has a repeated error message in the logs from an alert and a backtrace:

===

[09-10-31 23:14:49.2059] Slim::Utils::Misc::msg (1165) Warning: [23:14:49.2056] Backtrace:

   frame 0: Slim::Utils::Misc::assert (/<C:\PROGRA~1\SQUEEZ~2\server\SQUEEZ~3.EXE>Slim/Buttons/ScreenSaver.pm line 80)
   frame 1: Slim::Buttons::ScreenSaver::screenSaver (/<C:\PROGRA~1\SQUEEZ~2\server\SQUEEZ~3.EXE>Slim/Utils/Timers.pm line 255)
   frame 2: (eval) (/<C:\PROGRA~1\SQUEEZ~2\server\SQUEEZ~3.EXE>Slim/Utils/Timers.pm line 255)
   frame 3: Slim::Utils::Timers::__ANON__ (/<C:\PROGRA~1\SQUEEZ~2\server\SQUEEZ~3.EXE>Slim/Networking/IO/Select.pm line 183)
   frame 4: (eval) (/<C:\PROGRA~1\SQUEEZ~2\server\SQUEEZ~3.EXE>Slim/Networking/IO/Select.pm line 183)
   frame 5: Slim::Networking::IO::Select::loop (slimserver.pl line 620)
   frame 6: main::idle (slimserver.pl line 574)
   frame 7: main::main (slimserver.pl line 98)
   frame 8: PerlSvc::Interactive (/<C:\PROGRA~1\SQUEEZ~2\server\SQUEEZ~3.EXE>PerlSvc.pm line 99)
   frame 9: PerlSvc::_interactive (slimserver.pl line 0)
   frame 10: (eval) (slimserver.pl line 0)

Here's the problem. /<C:\PROGRA~1\SQUEEZ~2\server\SQUEEZ~3.EXE>Slim/Buttons/ScreenSaver.pm, line 80:

===
 
The save playlist plugin has coded in initPlugin ('right' hash key):  "setMode(.., 'playlist')". Shouldn't that just be popModeRight twice?
Comment 1 James Richardson 2009-11-01 12:58:21 UTC
These are handled by 3rd party developers, please report this error to the forums where you can get help from the developers.

The proper forum to report to is: http://forums.slimdevices.com/forumdisplay.php?f=4
Comment 2 James Richardson 2009-11-01 14:16:21 UTC
Phil: is this one yours to look at?

Richard:

If you disable all your 3rd-party plug-ins (SBS Control panel > stop SBS > Run with no user extensions> start SBS) does the same thing happen?
Comment 3 Philip Meyer 2009-11-01 14:36:46 UTC
Doesn't look to be anything to do with my PlaylistMan plugin.

The reporter mentioned the "Save Playlist" plugin, which is a Squeezebox Server plugin, I believe.  My PlaylistMan plugin copied the Save Playlist plugin code, it doesn't directly interact with Save Playlist in any way.

But it doesn't sound like an issue saving the playlist - it sounds like upon navigating into Song Info, the user can't get out.

rgrant - have you installed my PlaylistMan plugin?
Comment 4 rgrant 2009-11-02 12:14:01 UTC
This has to do with Plugin.pm under Slim\Plugin\SavePlaylist.

So this bug report is only related to the SBS Plugin SavePlaylist.

Disabling 3rd party plugins via safe mode also disables SBS Plugin SavePlaylist.

I restarted SBS in normal mode, and manually disabled all plugins except SavePlaylist. Navigated to Now Playing. Then followed the path of the OP, and the UI locked.

I copied the plugin from Slim\Plugin\SavePlaylist to Plugin\SavePlaylist. Made a few hacks, and tried popMode twice, and that kept the UI working. But the resultant mode is "home", not the source mode: "playlist". Pressing left arrow on the home menu bumps left (as it should). Adding pushMode 'playlist' works, but pressing the left arrow restores a SavePlaylist mode, not "home".

While the plugin can be modified to return to Now Playing, it never makes it back to 'playlist' with the UI flow intact.
Comment 5 rgrant 2009-11-03 09:56:28 UTC
Got mine working with two popMode($client) commands in place of the existing setMode command in the subref in initPlugin ('right' hash key). And a popMode at the start of setMode like:

Slim::Buttons::Common::popMode($client), return
  if ($push eq 'pop');

I found that the showBriefly shows up after the mode is switched, and is a little jerky UI wise, so the two pops might be better as a callback off showBriefly.
Comment 6 KDF 2009-11-03 16:34:14 UTC
I'll take this one.  I've reset the target for Jason to review as this is a CORE plugin problem.
Comment 7 KDF 2009-11-03 16:36:11 UTC
Created attachment 6268 [details]
back out after showBriefly

patch to properly back out of the save playlist mode after the showBriefly message.  Also tested for backing out without saving. Awaiting target to apply to 7.4.2 or 7.5.0.
Comment 8 James Richardson 2009-11-03 18:27:35 UTC
Thanks for the patch, please apply it to 7.4.2
Comment 9 SVN Bot 2009-11-03 18:35:26 UTC
 == Auto-comment from SVN commit #29149 to the slim repo by kdf ==
 == https://svn.slimdevices.com/slim?view=revision&revision=29149 ==

Bug: 15001
Description: back out of save playlist manually as setMode doesn't preserve browse tree.  Back out from a callback so that showBriefly can complete cleanly.
Comment 10 SVN Bot 2009-11-03 18:38:19 UTC
 == Auto-comment from SVN commit #29150 to the slim repo by kdf ==
 == https://svn.slimdevices.com/slim?view=revision&revision=29150 ==

bug: 15001 - changelog notice
Comment 11 KDF 2009-11-03 18:38:29 UTC
fixed for 7.4 trunk.  automerge should deal with 7.5 sometime tomorrow.  please reopen if there are any issues.
Comment 12 KDF 2009-11-14 16:18:23 UTC
breaks use via extras menu.
Comment 13 KDF 2009-11-14 16:19:50 UTC
*** Bug 15114 has been marked as a duplicate of this bug. ***
Comment 14 KDF 2009-11-14 16:20:06 UTC
will have to decide one vs the other.  Perhaps the plugin should no longer be on a menu outside of the play-hold function from now playing.
Comment 15 Chris Owens 2010-03-15 18:06:27 UTC
7.4.x milestone is in the past
Comment 16 Richard 2010-08-03 11:07:23 UTC
I use the menu mode.

The approach I've taken is to have "Save Playlist" and "Save Playlist 2" plugins where one handles the button approach and the other handles the menu approach.
Comment 17 Martin Williams 2011-11-25 07:09:51 UTC
(In reply to comment #12)
> breaks use via extras menu.

I resolve this locally by removing an extraneous 'popModeRight' from the
backspace action of 'savePluginCallback'. After doing that, breakage as
described in https://bugs-archive.lyrion.org/show_bug.cgi?id=15114 is cured.

And taking the same exit route when initially executed from Now Playing with
play.hold nows return to Now Playing, rather than backing up to the Home menu
(one menu too many).


Existing code:

sub savePluginCallback {
...
			
	} elsif ($type eq 'backspace') {

		Slim::Buttons::Common::popModeRight($client);
		Slim::Buttons::Common::popModeRight($client);
	
	} else {
...

Only one popModeRight is wanted.

Two pops were needed prior to SVN commit #29149. But that commit added an
additional 'popModeRight' when popping into 'setMode'. And this 'backspace'
action passes through that.

Added by #29149:

sub setMode {
...
	} elsif ($push eq 'pop') {

		# back out one more step since we've saved the playlist and are only partly backed out.
		Slim::Buttons::Common::popModeRight($client);
...
Comment 18 Martin Williams 2012-01-15 11:41:51 UTC
Created attachment 7608 [details]
Resolves use via extras menu (comment #12)

With reference to comment #17, attached proposed patch as outlined.

Tested on v7.6.1, also applies to server v7.7.
Comment 19 Michael Herger 2012-03-12 23:01:56 UTC
*** Bug 17924 has been marked as a duplicate of this bug. ***
Comment 20 SVN Bot 2012-03-13 00:25:11 UTC
 == Auto-comment from SVN commit #33893 to the slim repo by mherger ==
 == http://svn.slimdevices.com/slim?view=revision&revision=33893 ==

Fixed Bug: 15001
Description: Only one pop right is required despite needing to back up 2 levels.