Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-29864

Theme's with spaces in font-family incorrectly handled by minify library

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Trivial
    • Resolution: Fixed
    • Affects Version/s: 2.1.2, 2.2
    • Fix Version/s: 2.2
    • Component/s: Libraries, Themes
    • Labels:
      None
    • Testing Instructions:
      Hide
      • Modify /theme/standard/style/core.css to include a font-family of 'Trebuchet MS' (without quotes) - e.g. body {font-family:Trebuchet MS, Arial, Verdana, Helvetica, sans-serif;padding: 4px;}
      • Clear Caches
      • Ensure Theme Designed mode is not enabled
      • Reload the page
      • Use an inspector to view the CSS element.

      Expected result:
      font-family: Trebuchet MS,Arial,Verdana,Helvetica,sans-serif;

      Actual result:
      font-family: TrebuchetMS,Arial,Verdana,Helvetica,sans-serif;

      Show
      Modify /theme/standard/style/core.css to include a font-family of 'Trebuchet MS' (without quotes) - e.g. body {font-family:Trebuchet MS, Arial, Verdana, Helvetica, sans-serif;padding: 4px;} Clear Caches Ensure Theme Designed mode is not enabled Reload the page Use an inspector to view the CSS element. Expected result: font-family: Trebuchet MS,Arial,Verdana,Helvetica,sans-serif; Actual result: font-family: TrebuchetMS,Arial,Verdana,Helvetica,sans-serif;
    • Workaround:
      Hide

      Wrap any font which has spaces in it's name in quotes (e.g. Trebuchet MS becomes 'Trebuchet MS')

      Show
      Wrap any font which has spaces in it's name in quotes (e.g. Trebuchet MS becomes 'Trebuchet MS')
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-29864-master-2

      Description

      We've seen an issue where, if a theme contains a font-family declaration with a space in it, for example Trebuchet MS, it is minified to TrebuchetMS.

      This is already reported, and fixed in the upstream minify library under bug 210 (http://code.google.com/p/minify/issues/detail?id=210).
      Fixes 438 (Dec 2010) and 457 (June 2011) address this issue in the upstream library but there hasn't been a release since June 2009.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            dobedobedoh Andrew Nicols added a comment -

            Lowering priority because the workaround is a recommendation of the CSS specification in any case, and no upstream release is available with this fix.

            Show
            dobedobedoh Andrew Nicols added a comment - Lowering priority because the workaround is a recommendation of the CSS specification in any case, and no upstream release is available with this fix.
            Hide
            dobedobedoh Andrew Nicols added a comment -

            Since minify haven't released a new version in quite some time, but this is fixed in their upstream svn repository, I've picked the commit and applied it locally.

            This may affect theme developers who aren't aware of this bug.

            Tested under 2.1 and master.

            Show
            dobedobedoh Andrew Nicols added a comment - Since minify haven't released a new version in quite some time, but this is fixed in their upstream svn repository, I've picked the commit and applied it locally. This may affect theme developers who aren't aware of this bug. Tested under 2.1 and master.
            Hide
            dobedobedoh Andrew Nicols added a comment -

            dan_p pointed out that I hadn't updated the readme_moodle.txt to reflect the changes made

            Show
            dobedobedoh Andrew Nicols added a comment - dan_p pointed out that I hadn't updated the readme_moodle.txt to reflect the changes made
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Hi guys,
            Thanks for spotting and patching this.
            I've integrated this fix now however only on master (discussed this with Eloy).

            Cheers
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Hi guys, Thanks for spotting and patching this. I've integrated this fix now however only on master (discussed this with Eloy). Cheers Sam
            Hide
            phalacee Jason Fowler added a comment -

            code fix works exactly as described

            Show
            phalacee Jason Fowler added a comment - code fix works exactly as described
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Yes, you got this finally upstream, just in time for Moodle 2.2beta. Congrats and thanks!

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Yes, you got this finally upstream, just in time for Moodle 2.2beta. Congrats and thanks! Ciao

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  5/Dec/11