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

Error with preg_replace with theme css

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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

      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.

        Gliffy Diagrams

          Activity

          Hide
          electrolinux 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
          electrolinux 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
          skodak Petr Skoda added a comment -

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

          Petr

          Show
          skodak Petr Skoda added a comment - This should be fixed in the next weekly build, thanks for the report! Petr
          Hide
          scriby 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
          scriby 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:
                Fix Release Date:
                21/Feb/11