Moodle
  1. Moodle
  2. MDL-38000

CSS file names aren't handled consistently between request and delivery

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4.1
    • Fix Version/s: 2.3.5, 2.4.2
    • Component/s: Themes
    • Labels:
    • Testing Instructions:
      Hide
      1. Create a new theme based upon standard.
      2. Create a config file and within it define the minimum.
      3. Create a file theme/yourthemename/test.css and set to it: body {background-color:red !important}
      4. In your config file add "$THEME->sheets = array('../test');
      5. Browse to your site and switch to your theme.
      6. Enable theme designer mode if it is not already enabled.
      7. Purge your caches.
      8. Inspect the source for the CSS and check that the CSS from the test file was loaded correctly.
      Show
      Create a new theme based upon standard. Create a config file and within it define the minimum. Create a file theme/yourthemename/test.css and set to it: body {background-color:red !important} In your config file add "$THEME->sheets = array('../test'); Browse to your site and switch to your theme. Enable theme designer mode if it is not already enabled. Purge your caches. Inspect the source for the CSS and check that the CSS from the test file was loaded correctly.
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull 2.4 Branch:
      wip-MDL-38000-m24
    • Pull Master Branch:
      wip-MDL-38000-m25
    • Rank:
      47781

      Description

      This is an obscure we bug that I hit while looking at how to include CSS from a 3rd party library within the theme directory but not within the theme/style directory.

      To replicate:

      1. Create a new theme based upon standard.
      2. Create a config file and within it define the minimum.
      3. Create a file theme/yourthemename/test.css and set to it: body {background-color:red !important}
      4. In your config file add "$THEME->sheets = array('../test');
      5. Browse to your site and switch to your theme.
      6. Inspect the source for the CSS and note there was an issue.

        Activity

        Hide
        Sam Hemelryk added a comment -

        Up for peer-review now.

        Show
        Sam Hemelryk added a comment - Up for peer-review now.
        Hide
        Mary Evans added a comment -

        Works great.

        Show
        Mary Evans added a comment - Works great.
        Hide
        Mary Evans added a comment -

        I added my test stylesheet to style/css/test.css and added that location in config.php as $THEME->sheets = array('css/test');

        I presume that is the way we would need to do it correctly, wherever the location the CSS file is in.

        It does not work the way you wrote the instructions to replicate the problem, neither does it work with the patch unless you add the correct subdirectory.

        Hope this is what you want to hear?

        Show
        Mary Evans added a comment - I added my test stylesheet to style/css/test.css and added that location in config.php as $THEME->sheets = array('css/test'); I presume that is the way we would need to do it correctly, wherever the location the CSS file is in. It does not work the way you wrote the instructions to replicate the problem, neither does it work with the patch unless you add the correct subdirectory. Hope this is what you want to hear?
        Hide
        Mary Evans added a comment - - edited

        Sam, Your instruction for the test work OK, I have just realised that I misread the instructions last night, my mistake! I added the test.css to theme/themename/style/test.css instead of the theme/themename/test.css

        Show
        Mary Evans added a comment - - edited Sam, Your instruction for the test work OK, I have just realised that I misread the instructions last night, my mistake! I added the test.css to theme/themename/style/test.css instead of the theme/themename/test.css
        Hide
        Sam Hemelryk added a comment -

        Thanks Mary, no probs about the test instructions

        Putting this up for integration now.

        Show
        Sam Hemelryk added a comment - Thanks Mary, no probs about the test instructions Putting this up for integration now.
        Hide
        Mary Evans added a comment -

        I've Triaged it for you!

        Show
        Mary Evans added a comment - I've Triaged it for you!
        Hide
        Damyon Wiese added a comment -

        Integrated to master, 24 and cherry-picked to 23 (clean cherry pick and this is a low risk change).

        Show
        Damyon Wiese added a comment - Integrated to master, 24 and cherry-picked to 23 (clean cherry pick and this is a low risk change).
        Hide
        David Monllaó added a comment -

        It passes, works as expected in 24 and master

        Show
        David Monllaó added a comment - It passes, works as expected in 24 and master
        Hide
        Damyon Wiese added a comment -

        Thanks for your hard work - this issue has made it! Moodle is now a little bit better.

        Regards, Damyon

        Show
        Damyon Wiese added a comment - Thanks for your hard work - this issue has made it! Moodle is now a little bit better. Regards, Damyon

          People

          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: