Making CSS valid

Discuss PivotX 2.0.x here. Non-PivotX related discussions go in The Drain.

Making CSS valid

Postby Stingray » Tue Jan 24, 2012 9:44 pm

Hello PivotX developers,

I hope I put this in the right forum for possible inclusion in future releases of PivotX.

I don't know if you plan on including Thickbox in future releases of PivotX, but I've made a few changes so that it's css validates. First, I took all of the IE specific declarations and put them in their own css file (thickbox.ie.css). Next, I removed all of the -moz declarations; modern versions of Firefox are CSS3 compliant, so opacity works just fine by itself. Then, I needed to make changes to a couple of files so that the IE-specific declarations are loaded if IE is used.

To modules_extension.php:
Code: Select all
    $insert .= sprintf("\t<script type=\"text/javascript\" src=\"%sincludes/js/thickbox.js\"></script>\n",
        $PIVOTX['paths']['pivotx_url'] );
    $insert .= sprintf("\t<link rel=\"stylesheet\" href=\"%stemplates_internal/assets/thickbox.css\" type=\"text/css\" media=\"screen\" />\n",
        $PIVOTX['paths']['pivotx_url'] );
    // CHANGE I MADE
    $insert .= "\t<!--[if IE]>\n";
    $insert .= sprintf("\t<link rel=\"stylesheet\" href=\"%stemplates_internal/assets/thickbox.ie.css\" type=\"text/css\" media=\"screen\" />\n",
        $PIVOTX['paths']['pivotx_url'] );
    $insert .= "\t<![endif]-->\n";
    // END


To inc_header.tpl:
Code: Select all
    <script src="includes/js/thickbox.js" type="text/javascript"></script>
    <link rel="stylesheet" href="templates_internal/assets/thickbox.css" type="text/css" />
    <!-- CHANGE I MADE -->
    <!--[if IE]>
    <link rel="stylesheet" href="/pivotx/templates_internal/assets/thickbox.ie.css" type="text/css" media="screen" />
    <![endif]-->
    <!-- END -->


Finally, there was an underscore in thickbox.css (#TB_iframecontent, if I recall correctly) that needed to be removed.

Hope this is helpful!
Stingray
 
Posts: 107
Joined: Wed Oct 28, 2009 2:40 pm

Re: Making CSS valid

Postby Harm10 » Tue Jan 24, 2012 11:27 pm

Thanks for suggesting these fixes. But I am not sure whether we want to go this way. As you might know there is in fact not 1 browser called Internet Explorer but more than 1. :?
So are we going to cater for all these different versions (and have even more versions of the css)? I think we still are supporting IE7 so did you this the validity test with that version? I fear you are embarking on a difficult path.........

But again thanks for the information! :)
Perhaps the other developers have a different opinion..................... ;)
Quality is in the detail of things............

Want to change or update your PivotX site? Mail or PM me!
I can also convert your site to a Wordpress site!
Harm10
Developer
 
Posts: 2011
Joined: Wed Jun 17, 2009 9:37 am
Location: Somewhere in The Netherlands (aka Holland)

Re: Making CSS valid

Postby hansfn » Wed Jan 25, 2012 10:26 am

@Stingray: If you suggest code changes, please use a recent version of PivotX. We can't use your suggested changes to module_extensions.php. Fixing these CSS issues will be low priority, but if you can point at something that is broken and fixed by your changes ...
hansfn
Developer
 
Posts: 3282
Joined: Sun Nov 25, 2007 7:48 pm
Location: Molde, Norway

Re: Making CSS valid

Postby Stingray » Tue Jan 31, 2012 2:55 pm

Harm10 wrote:Thanks for suggesting these fixes. But I am not sure whether we want to go this way. As you might know there is in fact not 1 browser called Internet Explorer but more than 1. :?
So are we going to cater for all these different versions (and have even more versions of the css)? I think we still are supporting IE7 so did you this the validity test with that version? I fear you are embarking on a difficult path.........

But again thanks for the information! :)
Perhaps the other developers have a different opinion..................... ;)


I tried to keep it simple. I took all of the IE-specific declarations out of the main css file and put them in their own file. Why make every browser out there load declarations that are written for a specific browser. These were declarations that were already in the first css file, so I didn't feel it necessary to test something that should already have been tested. From what I've read, too, there isn't a version of IE that doesn't read the <!--[if IE]> stuff, and since they all would have loaded the css declarations, regardless of version, in the original css file, there should be no need to cater to the various versions out there, unless you're referring to browsers based on IE, such as Maxthon and Avant; I should see if those browsers obey the <!--[if IE]> statements.

Besides, with regard to testing, I use a linux machine, so I don't even use IE on my main machine. However, with these changes, the css validates according to w3c, if that's important to anyone.

hans, I know I'm not using a current version of PivotX. I've updated that one vulnerable file while waiting for the next version, which wasn't released when I wrote this, but is now. I haven't gone in to make this change in the 2.3.0, yet (I actually make a few other "hacks" for use on my site which I'm going through now, when I find the time). Still, this is a pretty simple change that could be adapted to any change made in module_extension as it's simply pulling IE-specific css out of one file, putting it in a new file, and placing <!--[if IE]> statements in the html to get IE to load the new file in IE browsers.

As for a fix, yes, there is a broken line in thickbox.css, which I mentioned in the original post; an underscore where one shouldn't be. I don't recall the exact line number, but it was near the end of the file.

Thanks for looking at this, though. I'm glad you at least take the time to look at changes others make to your code and respond.
Stingray
 
Posts: 107
Joined: Wed Oct 28, 2009 2:40 pm


Return to 2.x Discussion

Who is online

Users browsing this forum: No registered users and 3 guests