Confusing mythweb commit 289556

[jpasher]

I've decided to go through the mythweb code and do some cleanup (see viewtopic.php?f=2&t=3865), and I came across a strange commit. Commit 289556 has the following changelog:

------------------------------
commit 28955604496204af3c33b60089b9159ffbb96f0e
Author: David Hampton <mythtv@love2code.net>
Date: Fri Aug 30 12:06:39 2019 -0400

Fix the videos page CSS file name: video.css.php -> video.css.
------------------------------

It basically removed the video.css.php file from the different skins directories and changed the <link> tag to point directly to the CSS file (previously blank), with its content copied from the video.css.php file. However, that file had PHP directives in it to populate some of the values. The lines in modules/video/tmpl/default/video.php are now loading the CSS file twice: once dynamically and then statically.

$headers[] = '<link rel="stylesheet" type="text/css" href="'.root_url.'dcss/video.css">';
$headers[] = '<link rel="stylesheet" type="text/css" href="'.root_url.'skins/'.skin.'/video.css">';

Am I missing something obvious here as to why this was done? The functionality before basically said "load video.css.php as a dynamic CSS file, then load video.css as the static CSS file." Now it's saying "load video.css twice, first dynamically, then statically". It seems like the correct behavior is either revert to the old way (with separate files) or just always load video.css (once) dynamically. The latter seems more appropriate, as I'm not sure there is really a need to have both a static and dynamic file.

[hampton]

IIRC, I was trying to 1) eliminate a zero length file 2) align the name of an outlier CSS file with the names of all other CSS files. It sounds like I replaced one problem with a different problem. Please feel free to add any fix you think is appropriate to your open bug report.

[jpasher]

I guess there are technically two possible ways to fix it:

1. Remove the extra <link> referencing the css file directly
2. Remove the extra <link> referencing the css file directly AND rename video.css back to video.css.php (letting the dcss rewrite handle it)

Personally, I think it should be option 2. Since the CSS file contains PHP code, it should have a .php extension. Otherwise, if someone goes directly to the file, it doesn't process the PHP and shows the raw code (not necessarily a huge deal for the CSS file as-is, but not a good practice).