Moodle
  1. Moodle
  2. MDL-26153

Error with preg_replace with theme css

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.1
    • Fix Version/s: 2.0.2
    • Component/s: Themes
    • Labels:
      None
    • Difficulty:
      Moderate
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE
    • Rank:
      16174

      Description

      The preg_replace in theme/yui_combo.php does not take into account images that have numbers in their name. Example image: lib/yui/2.8.2/build/treeview/assets/skins/sam/treeview.css there are images check0.gif, check1.gif, check2.gif.

      The way I fixed it was to look for trailing numbers, but I don't think this is the best solution. Example:

      '/([a-z_-]+[0-9]*)\.(png|gif)/'

      I tried:

      '/([a-z_-0-9]+)\.(png|gif)/'

      But that didn't seem to work.

        Activity

        Hide
        didier Belot added a comment -

        Trying to understand the issue, it's difficult to at least see the bug in action. That's probably why a good bug report must include an easy 'Steps to reproduce'

        Anyway, looking at the source, the same regexp is used 3 times, but for the third use, it's preceded with another, bigger regexp sharing the same pattern near the end.
        So, if your first try, '/(a-z_-]+[0-9]*)\.(png|gif)/', which seems correct to me, was used in that third if/else branching, you have to use it 2 times:

        } else {
        // First we need to remove relative paths to images. These are used by YUI modules to make use of global assets.
        // I've added this as a separate regex so it can be easily removed once
        // YUI standardise there CSS methods
        $filecontent = preg_replace('#(\.\./\.\./\.\./\.\./assets/skins/sam/)?([a-z_-]+[0-9]*)\.(png|gif)#', '$2.$3', $filecontent);

        // search for all images in yui2 CSS and serve them through the yui_image.php script
        $filecontent = preg_replace('/([a-z_-]+[0-9]*)\.(png|gif)/', 'yui_image.php?file='.$version.'/$1.$2', $filecontent);

        Show
        didier Belot added a comment - Trying to understand the issue, it's difficult to at least see the bug in action. That's probably why a good bug report must include an easy 'Steps to reproduce' Anyway, looking at the source, the same regexp is used 3 times, but for the third use, it's preceded with another, bigger regexp sharing the same pattern near the end. So, if your first try, '/(a-z_-]+ [0-9] *)\.(png|gif)/', which seems correct to me, was used in that third if/else branching, you have to use it 2 times: } else { // First we need to remove relative paths to images. These are used by YUI modules to make use of global assets. // I've added this as a separate regex so it can be easily removed once // YUI standardise there CSS methods $filecontent = preg_replace('#(\.\./\.\./\.\./\.\./assets/skins/sam/)?( [a-z_-] + [0-9] *)\.(png|gif)#', '$2.$3', $filecontent); // search for all images in yui2 CSS and serve them through the yui_image.php script $filecontent = preg_replace('/( [a-z_-] + [0-9] *)\.(png|gif)/', 'yui_image.php?file='.$version.'/$1.$2', $filecontent);
        Hide
        Petr Škoda added a comment -

        This should be fixed in the next weekly build, thanks for the report!

        Petr

        Show
        Petr Škoda added a comment - This should be fixed in the next weekly build, thanks for the report! Petr
        Hide
        Chris Scribner added a comment -

        In this regex '/([a-z_-0-9]+)\.(png|gif)/', you need to escape the -, like this:

        '/([a-z_\-0-9]+)\.(png|gif)/'

        I think it's getting confused about it referring to the range _ through 0 instead of the character -.

        Chris

        Show
        Chris Scribner added a comment - In this regex '/( [a-z_-0-9] +)\.(png|gif)/', you need to escape the -, like this: '/( [a-z_\-0-9] +)\.(png|gif)/' I think it's getting confused about it referring to the range _ through 0 instead of the character -. Chris

          People

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

            Dates

            • Created:
              Updated:
              Resolved: