Moodle
  1. Moodle
  2. MDL-29686

yui_combo returns 304 HTTP header even when it is asked for a module with revision -1

    Details

    • Testing Instructions:
      Hide

      For developers:
      1/ disable js caching
      2/ edit some yui module fine in some moodle component (alert('grrr')
      3/ verify the modification is laoded automatically
      4/ enable caching
      5/ undo modification
      6/ verify the cached modified version is returned
      7/ reset caches
      8/ verify the original unmodified file is returned

      Show
      For developers: 1/ disable js caching 2/ edit some yui module fine in some moodle component (alert('grrr') 3/ verify the modification is laoded automatically 4/ enable caching 5/ undo modification 6/ verify the cached modified version is returned 7/ reset caches 8/ verify the original unmodified file is returned
    • Affected Branches:
      MOODLE_21_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      w40_MDL-29686_m2_yuicaching
    • Rank:
      19195

      Description

      Steps to reproduce:
      ===================

      1. make sure you have $CFG->cachejs = false;
      2. enable firebug or other favourite toy and watch requests for JS files via /theme/yui_combo.php
      3. locate a request that asks for a module with -1 revision like "http://your.url/moodle/theme/yui_combo.php?3.2.0/...blahblah...js&moodle/-1/block_navigation/navigation

      Expected result:
      ================

      As the block_navigation YUI module has revision -1 (thanks to cachejs = false), it should be reloaded and firebug should report result header "200 OK" with the reloaded content.

      What actually happens:
      ======================

      The script returns "304 Not Modified" header even when the module was modified. Hard reload must be performed.

        Activity

        Hide
        David Mudrak added a comment -

        Petr suggested the following patch and it seems to work very well for me:

        diff --git a/theme/yui_combo.php b/theme/yui_combo.php
        index 8d46a1d..a658d58 100644
        --- a/theme/yui_combo.php
        +++ b/theme/yui_combo.php
        @@ -48,7 +48,7 @@ if (substr($parts, -3) === '.js') {
         // if they are requesting a revision that's not -1, and they have supplied an
         // If-Modified-Since header, we can send back a 304 Not Modified since the
         // content never changes (the rev number is increased any time the content changes)
        -if (!empty($_SERVER['HTTP_IF_NONE_MATCH']) || !empty($_SERVER['HTTP_IF_MODIFIED_SINCE'])) {
        +if (strpos($parts, '/-1/') === false && (!empty($_SERVER['HTTP_IF_NONE_MATCH']) || !empty($_SERVER['HTTP_IF_MODIFIED_SINCE']))) {
             $lifetime = 60*60*24*30; // 30 days
             header('HTTP/1.1 304 Not Modified');
             header('Expires: '. gmdate('D, d M Y H:i:s', time() + $lifetime) .' GMT');
        
        
        Show
        David Mudrak added a comment - Petr suggested the following patch and it seems to work very well for me: diff --git a/theme/yui_combo.php b/theme/yui_combo.php index 8d46a1d..a658d58 100644 --- a/theme/yui_combo.php +++ b/theme/yui_combo.php @@ -48,7 +48,7 @@ if (substr($parts, -3) === '.js') { // if they are requesting a revision that's not -1, and they have supplied an // If-Modified-Since header, we can send back a 304 Not Modified since the // content never changes (the rev number is increased any time the content changes) - if (!empty($_SERVER['HTTP_IF_NONE_MATCH']) || !empty($_SERVER['HTTP_IF_MODIFIED_SINCE'])) { + if (strpos($parts, '/-1/') === false && (!empty($_SERVER['HTTP_IF_NONE_MATCH']) || !empty($_SERVER['HTTP_IF_MODIFIED_SINCE']))) { $lifetime = 60*60*24*30; // 30 days header('HTTP/1.1 304 Not Modified'); header('Expires: '. gmdate('D, d M Y H:i:s', time() + $lifetime) .' GMT');
        Hide
        Petr Škoda added a comment -

        +1, thanks!

        Show
        Petr Škoda added a comment - +1, thanks!
        Hide
        Sam Hemelryk added a comment -

        Hi Petr,
        This has been integrated now and has been cherry-picked to 21_STABLE.
        Worth noting is that this hasn't been cherry-picked to 20 as that branch is missing the 304 if statement.

        Cheers
        Sam

        Show
        Sam Hemelryk added a comment - Hi Petr, This has been integrated now and has been cherry-picked to 21_STABLE. Worth noting is that this hasn't been cherry-picked to 20 as that branch is missing the 304 if statement. Cheers Sam
        Hide
        Sam Hemelryk added a comment -

        Thanks guys - tested and passed

        Show
        Sam Hemelryk added a comment - Thanks guys - tested and passed
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Many thanks for the hard work developing and testing this. It has been spread to cvs and git upstream repositories.

        Closing, ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Many thanks for the hard work developing and testing this. It has been spread to cvs and git upstream repositories. Closing, ciao

          People

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

            Dates

            • Created:
              Updated:
              Resolved: