PDA

View Full Version : [PATCH] Prevent status frame from reloading all images



mcfly
2005-05-24, 01:23
> Anyway, after applying the patch things seem to work fine but as soon
> as the right pane reloads the track restarts which means I get about
> 30 seconds and then the song restarts.

Stupid me... I tested the patch without actually listening to the SB.
Here's a new version of the patch which should work (Yes, it listened to
the Box while testing it :-) ). With a playlist containing 10 entries,
the number of GET requests that the browser issues comes down from 43 to
11 with this patch.

Index: status_header.html
================================================== =================
--- status_header.html (revision 3282)
+++ status_header.html (working copy)
@@ -2,13 +2,25 @@
<head>
<title>[% "SQUEEZEBOX_MUSIC_PLAYER" | string %] [% IF player_name %][% player_name | html %][% END %]</title>
<meta http-equiv="Content-Type" content="text/html; charset=[% LOCALE %]">
- <meta http-equiv="refresh" content="[% refresh %]; url=status_header.html?player=[% player | uri %]&amp;start=[% start %]&amp;refresh=1">
<style type="text/css"> <!--
[% INSERT slimserver.css %]
--> </style>
<script language="JavaScript">
<!-- Start Hiding the Script

+function doLoad()
+{
+ setTimeout( "refresh()", [% refresh %]*1000);
+ if (parent.playlist.location != '') {
+ parent.playlist.location.reload(false);
+ }
+}
+
+function refresh()
+{
+ window.location.href="status_header.html?player=[% player | uri %]&amp;start=[% start %]&amp;refresh=1";
+}
+
function switchPlayer(player_List){
var player = player_List.options[player_List.selectedIndex].value;
parent.location.href = "[% webroot %]index.html?player=" + player;
@@ -17,7 +29,7 @@
// Stop Hiding script --->
</script>
</head>
-<body leftmargin="0" topmargin="0" marginwidth="0" marginheight="0" onload="if (parent.playlist.location != '') parent.playlist.location.reload(true);">
+<body leftmargin="0" topmargin="0" marginwidth="0" marginheight="0" onload="doLoad();">
<table width="100%" cellspacing="0">
<tr height="32">
<td width="4" class="darkgrey"></td>

mherger
2005-05-24, 01:32
Michel

this behaviour has been reported as a bug
(http://bugs.slimdevices.com/show_bug.cgi?id=1319)

> Stupid me... I tested the patch without actually listening to the SB.
> Here's a new version of the patch which should work (Yes, it listened to
> the Box while testing it :-) ). With a playlist containing 10 entries,
> the number of GET requests that the browser issues comes down from 43 to
> 11 with this patch.

Do you know why the browser does not request the images when reloaded
through the javascript? Makes no sense to me.

--

Michael

-----------------------------------------------------------
Help translate SlimServer by using the
StringEditor Plugin (http://www.herger.net/slim/)

Dan Sully
2005-05-24, 11:43
* Michel Marti shaped the electrons to say...

> > Anyway, after applying the patch things seem to work fine but as soon
> > as the right pane reloads the track restarts which means I get about
> > 30 seconds and then the song restarts.
>
>Stupid me... I tested the patch without actually listening to the SB.
>Here's a new version of the patch which should work (Yes, it listened to
>the Box while testing it :-) ). With a playlist containing 10 entries,
>the number of GET requests that the browser issues comes down from 43 to
>11 with this patch.

So I've tried this patch with and without my ETag patch. It works great for
Firefox. I'm still getting 200's with Safari either way.

IE 5.2 (OSX) works, IE6 (Win) works for the status_header, but doesn't for
the playlist frame (it fetches 200s).

-D
--
"A good messenger expects to get shot." --Larry Wall

mcfly
2005-05-24, 12:24
Dan Sully wrote:
> So I've tried this patch with and without my ETag patch. It works great for
> Firefox. I'm still getting 200's with Safari either way.
Hmm... Konqueror (3.3.0) doesn't send a "if-last-modified" and does not RE-GET
the images (I guess because it assumes correctly that the images have not yet
expired). Since Safari is based on KHTML I would expect that those two products
behave similar...


> IE 5.2 (OSX) works, IE6 (Win) works for the status_header, but doesn't for
> the playlist frame (it fetches 200s).
At least some improvement even with Ie6 ;-)

Dan Sully
2005-05-24, 13:09
* mma shaped the electrons to say...

> expired). Since Safari is based on KHTML I would expect that those two
> products behave similar...

I wouldn't go that far.. they have diverged.

> > IE 5.2 (OSX) works, IE6 (Win) works for the status_header, but doesn't for
> > the playlist frame (it fetches 200s).
>At least some improvement even with Ie6 ;-)

Yes. IE6 works completely correctly with my ETag patch and your status_header change.

-D
--
( ( ( [ ] ) ) )
In Stereo Where
Available

mcfly
2005-05-25, 03:04
Dan Sully wrote:
> Yes. IE6 works completely correctly with my ETag patch and your
> status_header change.

Looks like the current HTTP.pm is rather broken concerning the
generation of 304 responses. The code that should decide if a 304 can be
sent currently looks like this:

if (0 && $ifModified && $requestTime && $mtime)
^

Attached is a tiny patch that should improve the situation...

Dan Sully
2005-05-25, 07:22
* Michel Marti shaped the electrons to say...

>>Yes. IE6 works completely correctly with my ETag patch and your
>>status_header change.
>
>Looks like the current HTTP.pm is rather broken concerning the
>generation of 304 responses. The code that should decide if a 304 can be
>sent currently looks like this:
>
> if (0 && $ifModified && $requestTime && $mtime)
> ^
>
>Attached is a tiny patch that should improve the situation...

Right - that's been fixed. Please take a look at the latest 6.1 nightly,
which has your second patch, and my ETag patch applied.

-D
--
<noah> I used to be indecisive, but now I'm not sure.

mcfly
2005-05-25, 10:06
Dan Sully wrote:
> Right - that's been fixed. Please take a look at the latest 6.1 nightly,
> which has your second patch, and my ETag patch applied.

With the new refresh mechanism the browser's "back" history stacks up. The patch
below replaces location.href= with location.replace(...) which fixes this.


Index: status_header.html
================================================== =================
--- status_header.html (revision 3293)
+++ status_header.html (working copy)
@@ -18,7 +18,7 @@
}

function refresh() {
- window.location.href="status_header.html?player=[% player | uri %]&amp;start=[% start %]&amp;refresh=1";
+ window.location.replace("status_header.html?player=[% player | uri %]&amp;start=[% start %]&amp;refresh=1");
}

function switchPlayer(player_List){

Dan Sully
2005-05-25, 10:09
* mma shaped the electrons to say...

>>Right - that's been fixed. Please take a look at the latest 6.1 nightly,
>>which has your second patch, and my ETag patch applied.
>
>With the new refresh mechanism the browser's "back" history stacks up. The
>patch below replaces location.href= with location.replace(...) which fixes this.

Thanks - applied as subversion change 3294.

-D
--
<weezyl> $6.66: The Value Meal of the Beast.