Bug 4476 - browser specific stylesheets
: browser specific stylesheets
Status: RESOLVED WONTFIX
Product: Logitech Media Server
Classification: Unclassified
Component: Display
: 6.5.1
: Other Linux (other)
: -- enhancement (vote)
: Future
Assigned To: Unassigned bug - please assign me!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-11-06 10:39 UTC by Don Russell
Modified: 2012-03-12 23:30 UTC (History)
1 user (show)

See Also:
Category: ---


Attachments
fix comment error in slimserver.css file (248 bytes, text/plain)
2006-11-06 15:30 UTC, Don Russell
Details
fix comment error and display parameters in slimserver.css (748 bytes, text/plain)
2006-11-06 18:41 UTC, Don Russell
Details
fix comment error in slimserver.css file, tidy up other comments (512 bytes, text/plain)
2006-11-06 20:40 UTC, Don Russell
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Don Russell 2006-11-06 10:39:37 UTC
SlimServer Version: 6.5.1 - 10602 - RedHat - EN - utf8
Running on Fedora Core 6

I'm using the Firefox web browser to interface to Slimserver...

The javascript console has three slimserver.css errors which keep getting added everytime the web page refreshes.

Error: Expected declaration but found '/'.  Skipped to next declaration.
Source File: http://drussell.dnsalias.com:9000/slimserver.css
Line: 175

Error: Error in parsing value for property 'display'.  Declaration dropped.
Source File: http://drussell.dnsalias.com:9000/slimserver.css
Line: 472

Error: Error in parsing value for property 'display'.  Declaration dropped.
Source File: http://drussell.dnsalias.com:9000/slimserver.css
Line: 481

The first one is simple to solve.... it is an incorrectly coded comment
A snip from the css file shows this...

.PrefExtValue {
//      width: 200px;      <--- line 175 - error
        width: 80%;
}

Double slash is not a valid comment in css... comments are started with /* and terminated with */
So, if the intent is to comment out an earlier declaration...
/* width: 200px; */
is the correct form.

The next two are a little more tricky.... :-)

.thumbwrap {
        display: inline-block;     <-- line 472 - error, hmm valid css, Firefox bug
        margin: 0px;
        padding-left: 4px;
        padding-right: 0px;
        padding-top: 10px;
}

.thumbwrap li {                     <-- line 479 (see below)
        display: -moz-inline-box;  /* Moz */
        display: inline-block;  /* Op, Saf, IE \*/    <-- line 481, same error as line 472
        vertical-align: top;
        padding-right: 6px;
        padding-top:6px;
}


So, the errors relating to display: inline-block are "acceptable"... I'll see where I get opening a bug against FireFox...

But, regarding line 479 --> /.thumwrap li {

Are you trying to apply the styles to elements with a class of "thumbwrap" and <li> tags? Or perhaps only li tags of class thumbwrap?
For the former, the list the styles appply to need to be separated by a comma: --> .thumbwrap, li {
and for the later --> li.thumbwrap {

Rather than coding browser-specific values in the css file, use a different approach:
1 - (prefered) - the server can add a <style type="text/css">...</style> section for all browser specific values
2 - (acceptable) - a javascript function performed by the client to apply browser-specific styles....


Thanks,
Don Russell
Comment 1 KDF 2006-11-06 11:41:54 UTC
diff -upB welcome.
Comment 2 Don Russell 2006-11-06 15:30:19 UTC
Created attachment 1692 [details]
fix comment error in slimserver.css file

This patch fixes only the comment errorin the css file.... perhaps it can be included in the "daily build" for 6.5.1

I'llfix the other errors, but those take more time because I need to understand the intention of the code.... and find the correct server component so it builds the brower-specific styles.

However, this will reduce my javascript error log by 1 third. ;-)

Thanks
Comment 3 Don Russell 2006-11-06 18:41:14 UTC
Created attachment 1693 [details]
fix comment error and display parameters in slimserver.css

I've created a new diff file that corrects all three errors.

It appears lines 472 and 481 are already being taken care of in a <style> element produced following the <style src=...> elements, and so are not needed in this file.
Comment 4 KDF 2006-11-06 20:13:02 UTC
sadly, while this works with IE and clears the trivial warning reports from Firefox, it does break Safari.  That's a show-stopper really. 

do you know of an equivalent of <!--[if IE ]... for firefox or safari?

if so, that could be used to handle the special cases to make sure the "gallery" view shows up in a proper grid layout as designed.

Comment 5 Don Russell 2006-11-06 20:40:48 UTC
Created attachment 1694 [details]
fix comment error in slimserver.css file, tidy up other comments

Darn! 

OK... let me look at this some more... I know *what* to do, I just don't know *where* to do it. :-(

slimserver is in /usr/local/slimserver but I can't find the source files....

I've backed out the "show stoppers" and attached a new diff file for you.... can we at least go with this.. it cuts down a third of the errors. :-)

If I can find the part of the code that writes out the html <head> section... this shouldn't be difficult to solve nicely. Can you point me to the correct files?

Thanks
Comment 6 KDF 2006-11-06 20:54:45 UTC
dude, I'm so with you.  I ended up giving up based on "looks good out front, ignore the console".

The hacks that you've noticed making things work are located in pageheader.html, using <!-- [if IE.., which takes care oof I but I never found any great solution for handling safari inline to the css.  I know that javascript can be used to detect and optionally load various css, but this has other issues (not to mention more maintenance issues with multiple files).  

I'm guessing that you've been using Default skin, which is fine as I can always port changes over.  The specific location for the file you need is /usr/local/slimserver/HTML/Default/pageheader.html

thanks for your help.
Comment 7 Don Russell 2006-11-06 21:44:48 UTC
(In reply to comment #6)

> I'm guessing that you've been using Default skin, which is fine as I can always
> port changes over.  The specific location for the file you need is
> /usr/local/slimserver/HTML/Default/pageheader.html
> 
> thanks for your help.

OK... now, where do I find the code that does 
[% PROCESS standardheader.html ]

I want to get to the code that is actually creating the http response.... the server can look at the http-user-agent: request header and, ideally include the necessary <link> tag to pull in a browser-specific css file.

Or, write an "in-line" <style>...</style> definition... 

Or, as you mentioned, use javascript.. specifically look at the window.navigator object

Well.. that's it for me for today... :-(

Thanks for the speedy conversation. :-)

Comment 8 KDF 2006-11-06 21:59:17 UTC
the inline <style> is in pageheader.html.  standardheader.html is another file that, by the use fo [% PROCESS... %] gets included as part of pageheader.html through template toolkit ([% PROCESS %] is a TT directive).  

Comment 9 Don Russell 2006-11-06 23:28:24 UTC
(In reply to comment #8)
> the inline <style> is in pageheader.html.  standardheader.html is another file
> that, by the use fo [% PROCESS... %] gets included as part of pageheader.html
> through template toolkit ([% PROCESS %] is a TT directive).

OK... I'm proposngwe remove the <style> from pageheader.html, and modify something further "upstream" where the headers are written.

Where doI find the php code that does that?
Comment 10 KDF 2006-11-06 23:58:03 UTC
heh...there be no php here mate :)
it be perl, Slim/Web/Pages.pm in this case, wiht Slim/Web/HTTP.pm being the core web server (where headers are provided)

trust me, you'll want to do it in the html/templates if there is any way.
Comment 11 Don Russell 2006-11-07 00:32:58 UTC
lol... where did I get the idea this was all written in php? - No need to answer :-) 

OK... this is getting interestinger and interestinger. :-)

I'll see what I can figure out.... unless I have more "where is ... questions", it'll probably be a little while before I get something working and get you an upate.... 
Comment 12 KDF 2006-11-07 00:50:35 UTC
I'm all ears.  you probably thought php cause it's sane ;)
as any questions on the developer forum if you are interested in digging into this. I'm no expert, just the hack who tried and erred through much of the css (and still doesn't get it).  it would be great to sort this stuff out.
Comment 13 KDF 2007-05-17 16:41:25 UTC
setting as an enhancement to review in future. There are just too many quirks that cause warning output among browsers so the solution is really to have sets of css to load specific to each browser where necessary.  For now, console output should be accepted as a necessary evil.
Comment 14 Don Russell 2007-05-17 19:01:44 UTC
Something else that might be acceptable, and easier to accomplish:

It appears the css files are retrieved from the the server repeatedly. i.e. with each page refresh while audio is streaming.

How much work is it to include a "Last-Modified:" response header so the browser will send an "If-Modified-Since:" request header? That way SlimServer can reply with a 304 Not Modified, which accomplishes two things:

1 - reduction of duplicate stuff sent over the network (the css files don't change during a session)
2 - the errors/warning should only be issued once because the browser isn't likely to parse the file all over again

Even if "2" turns out to be not true, it's worth it for benefit "1".

The reason I would like the errors/warnings corrected is because I develop other pages while SlimServer is playing... when I look at the console to check out errors in *my* stuff, I'm overwhelmed by all the repeated errors from SlimServer...

I do agree, browser specific css files is the best solution... I just wanted to add this for "consideration". :-)

Thanks,
Don
Comment 15 Don Russell 2007-12-30 07:50:22 UTC
While working on another (unrelated) CSS project I used a different approach to CSS files.

In the header I used this method:
	<link href="css/css.css" rel="stylesheet"/>	

	<!--[if IE]>
	<link href="css/msie.css" rel="stylesheet"/>	
	<![endif]-->

Two CSS files.... the first has all the "standard" CSS which seems to work on browsers like Firefox, Opera and Safari.
The second, which is only looked at by IE contains the quirky things needed by IE.

This method has a couple of advantages over "weird" hacks in the CSS file itself:
- The CSS file(s) are kept cleaner.
- it's easy to understand.
- doesn't rely on a bug in one browser to fix something in another

The server doesn't need to know which browser is involved. Only IE will look at the html between <!--[if IE] ... [endif]-->

MSIE also supports finer granularity than that if needed... for example, you do not have to match on exact version numbers of IE.
Ref. http://www.quirksmode.org/css/condcom.html for more information.

I like this method, because as that article explains, it "does not rely on one browser's bug to fix another".

I just wanted to document this info with this bug for future reference. :-)

Comment 16 James Richardson 2009-01-14 11:55:30 UTC
Is this even still an issue, now that SC 7.x.x is released?
Comment 17 Alan Young 2011-11-06 23:23:40 UTC
Unassigned bugs cannot have a priority.
Comment 18 Michael Herger 2012-03-12 23:30:54 UTC
While we do have browser specific stylesheets, there will never be an all clean console.