Bug 6949 - InfoBrowser applet fails when no players, and when server not connected
: InfoBrowser applet fails when no players, and when server not connected
Status: CLOSED FIXED
Product: SB Controller
Classification: Unclassified
Component: UI
: unspecified
: All All
: -- normal (vote)
: 7.0.1
Assigned To: Adrian Smith
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-02-05 04:52 UTC by Matthew Flint
Modified: 2009-09-08 09:13 UTC (History)
2 users (show)

See Also:
Category: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Matthew Flint 2008-02-05 04:52:11 UTC
Hello,

Two small problems with the InfoBrowser applet:

1. if there's no player, the applet will connect to the first server in the list. This server isn't necessarily available.

Suggest changing this:
--------
for _, server in sd:allServers() do
    self.server = server
    break
end
--------
to:
--------
for _, server in sd:allServers() do
    if (server:isConnected()) then
        self.server = server
        break
    end
end
--------

2. if there's no player, but valid server is found, the comet requests are still sent with "self.player.id".

Suggest changing the comet requests so "self.player.id" is replaced with "nil" or "self.player and self.player.id". (Don't see why a player is necessary for these InfoBrowser requests)
Comment 1 Blackketter Dean 2008-02-05 07:19:52 UTC
What do you think, Triode?
Comment 2 Andy Grundman 2008-02-05 09:26:45 UTC
I suggest for 7.0.1 we make InfoBrowser a SC-only feature, not a Jive applet.  It should not appear unless you're connected to a player.
Comment 3 Adrian Smith 2008-02-05 12:34:27 UTC
I agree this should be fixed for 7.0 as it can cause a crash

Patch is:

--- src/share/applets/InfoBrowser/InfoBrowserApplet.lua 2008-02-01 19:58:03.000000000 +0000
+++ applets/InfoBrowser/InfoBrowserApplet.lua   2008-02-05 20:29:05.000000000 +0000
@@ -66,8 +66,10 @@
                        self.server = self.player:getSlimServer()
                else
                        for _, server in sd:allServers() do
-                               self.server = server
-                               break
+                               if server:isConnected() then
+                                       self.server = server
+                                       break
+                               end
                        end
                end
        end
@@ -90,7 +92,7 @@
                                self:response(chunk.data, window, widget, list, prevmenu, locked)
                        end
                end,
-               self.player.id,
+               self.player and self.player.id,
                { 'infobrowser', 'items', start, gulp, index and ("item_id:" .. index) }
        )
 end


Note this does not allow Infobrowser to work without a player attached - this appears because the server comet session does not work with no player...  I don't see why we need to do this?

Dean - do you want the above in 7.0?
Comment 4 Blackketter Dean 2008-02-05 13:19:58 UTC
Yes, please!

Thanks, Triode.

-dean
Comment 5 Adrian Smith 2008-02-05 14:10:00 UTC
patch added as r1822

Keeping bug open to look at SC only version suggested by Andy
Comment 6 Richard Titmuss 2008-03-11 14:02:45 UTC
Making InfoBrowser SC only should be a 7.1 fix.
Comment 7 Adrian Smith 2008-03-11 14:08:27 UTC
Actually I'd prefer to have a mechanism in 7.1 to allow an applet to be removed from the menu if the current server does not support it (e.g. its SN).  

This would allow other applets such as the network test one to be included and not show up with SN - which I believe is at the root of Andy's requirement?  Otherwise we are going to push for all addons to be SC plugins rather than applets so they don't show up with SN...
Comment 8 Blackketter Dean 2008-03-11 16:34:21 UTC
Triode: do you think that this should be a 7.0.1 addition?

Comment 9 Adrian Smith 2008-03-11 16:46:49 UTC
If 7.0.1 is going to live for any significant time and you want people to add applets then yes I tend to think so..  
Comment 10 Blackketter Dean 2008-03-11 16:51:44 UTC
It will and I do.  

Patches?
Comment 11 Adrian Smith 2008-03-12 09:20:45 UTC
Looks like we can already make an applet appear in the menu conditional on which server is connected - does this seem the right thing to do?

function registerApplet(meta)
	jnt:subscribe(meta)

	meta.menu = meta:menuItem('appletNetTest', 'advancedSettings', meta:string('NETTEST'), function(applet, ...) applet:open(...) end, 10)

	jiveMain:loadSkin("NetTest", "skin")
end


function notify_playerCurrent(meta, player)
	if player == nil or player.slimServer.name == 'SqueezeNetwork' then
		jiveMain:removeItem(meta.menu)
	else
		jiveMain:addItem(meta.menu)
	end
end
Comment 12 Richard Titmuss 2008-03-12 09:24:36 UTC
That looks good, small change:
  player.slimServer.name == 'SqueezeNetwork'

should be:
  player.slimServer.isSqueezeNetwork()

If we find that lots of applets need this then we probably should push this into the Home menu api.
Comment 13 Adrian Smith 2008-03-12 10:26:04 UTC
change 2104 - detect lack of player and remove from the menu

I prefer this to making infobrowser a SC plugin so consider this fixed...
Comment 14 Matthew Flint 2008-03-13 01:11:21 UTC
Good work - thanks chaps :-)

Matthew
Comment 15 James Richardson 2008-05-06 15:05:43 UTC
Closing bug in 7.0.1