Bug 13691 - NP context menu should be able to remove items without munging the menu list callbacks
: NP context menu should be able to remove items without munging the menu list ...
Status: CLOSED FIXED
Product: SqueezePlay
Classification: Unclassified
Component: Now Playing
: unspecified
: PC Windows Vista
: P1 normal with 1 vote (vote)
: 7.5.0
Assigned To: Ben Klaas
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-08-26 09:06 UTC by Sue Chastain
Modified: 2010-04-08 17:24 UTC (History)
4 users (show)

See Also:
Category: ---


Attachments
fix for NP CM submenus not passing extra params to server (496 bytes, patch)
2010-03-08 13:00 UTC, Ben Klaas
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sue Chastain 2009-08-26 09:06:23 UTC
When the context menu is opened from the playlist, the menu items all seem to work (I didn't try all of them, but the ones I did try worked). But when the context menu is invoked from the Now Playing screen, then some context menu items don't work. Love this track, comments, lyrics, more info, all resulted in an endless spinny icon. Browse-able items like artist, album, year seemed to work, but I didn't check them all. Maybe someone who's on the payroll can QC them all using both methods.

Version: 7.4 - r28276
Controller r7262
Comment 1 Ben Klaas 2009-08-26 09:34:42 UTC
Thanks Sue, and sorry we can't seem to get this right :(
Comment 2 Ben Klaas 2009-08-27 21:26:01 UTC
*** Bug 13735 has been marked as a duplicate of this bug. ***
Comment 3 Mikael Nyberg 2009-08-27 21:44:53 UTC
Vote don't work, I get no option to vote on this one.
Sorry for the dupe search bugzilla is hopeless, this one was probably easy to find but in generall you won't find anything (or rather everything ) so you stop using search.
Comment 4 Ben Klaas 2009-08-27 21:48:19 UTC
Mikael- never worry about posting a duplicate bug. We'd much rather have a bug posted twice than not at all. :)
Comment 5 Ben Klaas 2009-08-28 07:38:34 UTC
SP sending this command for "Love this Track" when accessing CM from NP screen
{ --[[table: 0x174f1320]]
  "trackinfo",
  "items",
  0,
  200,
  "menu:trackinfo",
  "item_id:0",
  "useContextMenu:1",
}

Command sent from "Love this Track" when accessing CM via playlist window
{ --[[table: 0x17428e40]]
  "trackinfo",
  "items",
  0,
  200,
  "item_id:0",
  "menu:trackinfo",
  "track_id:7111",
  "useContextMenu:1",
}

so, missing the track_id tag to the command. now to tease out why that's happening...problem is definitely on the server side.
Comment 6 SVN Bot 2009-08-28 07:58:53 UTC
 == Auto-comment from SVN commit #28324 to the slim repo by bklaas ==
 == https://svn.slimdevices.com/slim?view=revision&revision=28324 ==

Fixed Bug: 13691
Description: Push track_id and url params into request object so sub-menus have the right params
Comment 7 Ben Klaas 2009-08-28 08:08:11 UTC
The first order bug is fixed, but Last.Fm scrobbling item isn't actually working (though it now _looks_ like it's working). Filed bug 13741 against the second order bug.
Comment 8 James Richardson 2009-10-05 14:33:01 UTC
This bug has been marked as fixed in the 7.4.0 release version of SqueezeBox Server!
    * SqueezeCenter: 28672
    * Squeezebox 2 and 3: 130
    * Transporter: 80
    * Receiver: 65
    * Boom: 50
    * Controller: 7790
    * Radio: 7790  

Please see the Release Notes for all the details: http://wiki.slimdevices.com/index.php/Release_Notes

If you haven't already, please download and install the new version from http://www.logitechsqueezebox.com/support/download-squeezebox-server.html

If you are still experiencing this problem, feel free to reopen the bug with your new comments and we'll have another look.
Comment 9 Mikael Nyberg 2010-02-27 01:33:03 UTC
Hi I want to reopen this regarding controller 7.5.0r8587

Directly on the NP screen (playlist works just fine):

Some, not all submenus are broken, here the one's I tested.

"more info "= empty

"on amazon"= empty

"on last fm radio" = empty

"share on facebook" = more info !?!?!

I have the playlist manager plugin whos menu is always broken (but that's an issue with that plugin)

I have to stress that the submenus work from the playlist.

The difference from the first time we run this bug was, that now you "empty" instead of endless spinning circle.

How do I get this reopened and confirmed with the new rules ? I don't have the magic wand ;)
Comment 10 Ben Klaas 2010-02-27 09:04:23 UTC
this has been seen again. flag for Monday bug meeting discussion.
Comment 11 Mikael Nyberg 2010-02-28 03:53:28 UTC
I works on Radio so this may be controller only ?
Have no Touch can not test there.
Comment 12 Mikael Nyberg 2010-02-28 03:57:02 UTC
gaah I take that back I faults on the radio too.
It worked once ?
Comment 13 Ben Klaas 2010-03-01 07:54:35 UTC
FYI, I have a handle on this...I caused the regression by checking in my fix for bug 13787.

Not sure what the fix is that will cover both of them just yet, but fairly certain that's where the problem lies.

This is a P1.
Comment 14 Ben Klaas 2010-03-01 08:27:16 UTC
confirmed, that's the cause of the regression.

In Slim::Menu::TrackInfo, I'm returning an empty set for when TrackInfo is called for the current track:

sub playTrack {
        my ( $client, $url, $track, $remoteMeta, $tags ) = @_;
        my $items = [];
        my $jive;

        my $play_string = cstring($client, 'PLAY');

        my $actions;

        # Play is not a valid item in the context menu of the currently playing track
        if ( $tags->{menuContext} eq 'playlist' && $tags->{currentTrack} ) {
                return $items;
        }


this has the effect of removing the item from the menu but then all of the callbacks for other items are misaligned and the cli requests for them fail (e.g., "On Flickr" returned data for me for "On Pandora", while others failed entirely)
Comment 15 Andy Grundman 2010-03-02 12:03:33 UTC
Ben to look at having SP always pass the necessary tagged params when in the NP 
context menu.
Comment 16 SVN Bot 2010-03-02 12:50:43 UTC
 == Auto-comment from SVN commit #8612 to the jive repo by bklaas ==
 == https://svn.slimdevices.com/jive?view=revision&revision=8612 ==

Fixed Bug: 13691
Description: inject context = 'playlist' and currentTrack = '1' into the params of any NP CM item
Comment 17 Mikael Nyberg 2010-03-04 11:20:16 UTC
Heollo don't forget the Radio it has the same problem, but you are not building any new software for it it's stuck on:

baby_7.5.0_r8594.bin

This still has the bug :-/
Comment 18 Ben Klaas 2010-03-04 11:27:43 UTC
FYI, the code fix will work for all Squeezeplay platforms, including radio.

Radio goes through extra QA before being pushed out so we don't brick players in the field. The next update will clear the issue.
Comment 19 Mikael Nyberg 2010-03-06 12:21:36 UTC
Sorry to say this is back but now in the sub/sub menu If you get me ?

in the NP screen (NP play list works just fine )

Go to for example "On Amazon" then your beeing presented with artist & album,
press any of these menu's and you get "Empty" and on the top of the screen "userdata:0x6ef1e0" 

Similar errors on the other functions sub menu is ok but the menu below is not ok

Tried with last-FM and facebook
Comment 20 Ben Klaas 2010-03-08 10:55:32 UTC
confirmed. submenus are not identified by step._isNPChildWindow so don't get the necessary tags, which breaks the XMLBrowse callbacks.

flagging for bug_meeting for discussion. Hope to have some sort of patch to attach before then.
Comment 21 Ben Klaas 2010-03-08 13:00:41 UTC
Created attachment 6610 [details]
fix for NP CM submenus not passing extra params to server

if a submenu of a NP CM is given the _isNpChildWindow flag based on the fact that its origin (the menu above it) also had that flag, this bug is fixed.

this is a one line fix and has been tested. Very strong recommendation from me to include it for 7.5.0. IMO the only other option is to revert a more extensive checkin from a few weeks ago, which will clear the regression but reopen a different bug.

Leaving this bug in its current state is broken.
Comment 22 SVN Bot 2010-03-09 08:27:39 UTC
 == Auto-comment from SVN commit #8641 to the jive repo by bklaas ==
 == https://svn.slimdevices.com/jive?view=revision&revision=8641 ==

Fixed Bug: 13691
Description: children of NP child windows are also NP child windows
Comment 23 SVN Bot 2010-03-11 08:53:25 UTC
 == Auto-comment from SVN commit #30365 to the slim repo by bklaas ==
 == https://svn.slimdevices.com/slim?view=revision&revision=30365 ==

Bug: 13787
Bug: 13691
Description: revert changes to fix bug 13787. Adding tags to XMLBrowse request is not a good road to go down.
Comment 24 SVN Bot 2010-03-11 08:54:29 UTC
 == Auto-comment from SVN commit #8651 to the jive repo by bklaas ==
 == https://svn.slimdevices.com/jive?view=revision&revision=8651 ==

Bug: 13787
Bug: 13691
Description: revert changes to fix bug 13787. Adding tags to XMLBrowse request
is not a good road to go down.
Comment 25 Chris Owens 2010-04-08 17:24:34 UTC
This bug has been marked fixed in a released version of Squeezebox Server or the accompanying firmware or mysqueezebox.com release.

If you are still seeing this issue, please let us know!