Moodle
  1. Moodle
  2. MDL-30659

Serve non-minified YUI3 in developer debug mode

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.6, 2.1.3, 2.2
    • Fix Version/s: 2.1.4, 2.2.1
    • Component/s: Libraries
    • Labels:
    • Testing Instructions:
      Hide

      1/ install firebug in FF
      2/ enable Net tab
      3/ review the YUI2 and 3 CSS+JS in normal and dev debug mode
      4/ edit the outputrequirements.php $this->yui3loader->filter = YUI_DEBUG;, test that JS console shows YUI debug and loads debug files

      Show
      1/ install firebug in FF 2/ enable Net tab 3/ review the YUI2 and 3 CSS+JS in normal and dev debug mode 4/ edit the outputrequirements.php $this->yui3loader->filter = YUI_DEBUG;, test that JS console shows YUI debug and loads debug files
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      w51_MDL-30659_m23_yuidebug
    • Rank:
      33476

      Description

      I thought we had already implemented this, except that JS served via theme/yui_combo.php was still coming through as minified, so we need to do more.

        Issue Links

          Activity

          Hide
          Petr Škoda added a comment -

          hmm, should not this be done only when yui is not told to cache stuff? otherwise switching on/off the debug mode will not work when caching enabled, right?

          Show
          Petr Škoda added a comment - hmm, should not this be done only when yui is not told to cache stuff? otherwise switching on/off the debug mode will not work when caching enabled, right?
          Hide
          Tim Hunt added a comment -

          Petr, that is exactly why I made you a watcher here! Thank you.

          Becuase of ABORT_AFTER_CONFIG, the un-minified YUI will only be sent if $CFG->debug = 38911 is set in config.php. Would that ever happen on a production server?

          If this happens on a developers' server, would it really matter?

          If you think it is better, I could add $cache = false; inside the new if statement. What do you think is best.

          Also, it would be really nice to avoid hard-coding 38911, but I can't think of a way to achieve that.

          Note that this change is useful. It let me work out http://yuilibrary.com/projects/yui3/ticket/2531561

          Show
          Tim Hunt added a comment - Petr, that is exactly why I made you a watcher here! Thank you. Becuase of ABORT_AFTER_CONFIG, the un-minified YUI will only be sent if $CFG->debug = 38911 is set in config.php. Would that ever happen on a production server? If this happens on a developers' server, would it really matter? If you think it is better, I could add $cache = false; inside the new if statement. What do you think is best. Also, it would be really nice to avoid hard-coding 38911, but I can't think of a way to achieve that. Note that this change is useful. It let me work out http://yuilibrary.com/projects/yui3/ticket/2531561
          Hide
          Sam Hemelryk added a comment -

          Hi guys,

          I was just having a look at this now, I was sure that this was working in the past.
          A bit of research and indeed it was working in the past but due to changes in the YUI loader, or perhaps just in YUI it no longer does.
          The solution appears to be pretty simple:

          diff --git a/lib/outputrequirementslib.php b/lib/outputrequirementslib.php
          index 93c5cbe..2401862 100644
          --- a/lib/outputrequirementslib.php
          +++ b/lib/outputrequirementslib.php
          @@ -160,7 +160,7 @@ class page_requirements_manager {
                   $this->M_yui_loader->base         = $this->yui3loader->base;
                   $this->M_yui_loader->comboBase    = $this->yui3loader->comboBase;
                   $this->M_yui_loader->combine      = $this->yui3loader->combine;
          -        $this->M_yui_loader->filter       = ($this->yui3loader->filter == YUI_DEBUG) ? 'debug' : '';
          +        $this->M_yui_loader->filter       = $this->yui3loader->filter;
                   $this->M_yui_loader->insertBefore = 'firstthemesheet';
                   $this->M_yui_loader->modules      = array();
                   $this->M_yui_loader->groups       = array(
          

          Basically it looks like things have changed from requiring the filter to be defined as 'debug' to being defined as one of 'DEBUG', or 'RAW' for which we can use the YUI defines YUI_DEBUG, and YUI_RAW.
          In our case we have traditionally used raw and I have left it at that.

          Would be great if someone else could test this.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi guys, I was just having a look at this now, I was sure that this was working in the past. A bit of research and indeed it was working in the past but due to changes in the YUI loader, or perhaps just in YUI it no longer does. The solution appears to be pretty simple: diff --git a/lib/outputrequirementslib.php b/lib/outputrequirementslib.php index 93c5cbe..2401862 100644 --- a/lib/outputrequirementslib.php +++ b/lib/outputrequirementslib.php @@ -160,7 +160,7 @@ class page_requirements_manager { $ this ->M_yui_loader->base = $ this ->yui3loader->base; $ this ->M_yui_loader->comboBase = $ this ->yui3loader->comboBase; $ this ->M_yui_loader->combine = $ this ->yui3loader->combine; - $ this ->M_yui_loader->filter = ($ this ->yui3loader->filter == YUI_DEBUG) ? 'debug' : ''; + $ this ->M_yui_loader->filter = $ this ->yui3loader->filter; $ this ->M_yui_loader->insertBefore = 'firstthemesheet'; $ this ->M_yui_loader->modules = array(); $ this ->M_yui_loader->groups = array( Basically it looks like things have changed from requiring the filter to be defined as 'debug' to being defined as one of 'DEBUG', or 'RAW' for which we can use the YUI defines YUI_DEBUG, and YUI_RAW. In our case we have traditionally used raw and I have left it at that. Would be great if someone else could test this. Cheers Sam
          Hide
          Tim Hunt added a comment -

          Aha! Thanks I will look at this tomorrow.

          In the past, we used to serve either debug or min YUI, but that was a real pain, because mostly yui-debug fills the javascript console with crap that dose not tell you anything useful, but which hides the real error.

          So we changed it to yui plain or min, but that line of code you point out probably needed to be changed and was not.

          Show
          Tim Hunt added a comment - Aha! Thanks I will look at this tomorrow. In the past, we used to serve either debug or min YUI, but that was a real pain, because mostly yui-debug fills the javascript console with crap that dose not tell you anything useful, but which hides the real error. So we changed it to yui plain or min, but that line of code you point out probably needed to be changed and was not.
          Hide
          Petr Škoda added a comment -

          Ah, the yui3 loader is not used because it did not support the latest yui3 - I had to include the necessary JS manually and I forgot to add exception for the non-minified stuff. I will fix it somehow soon, thanks!

          Show
          Petr Škoda added a comment - Ah, the yui3 loader is not used because it did not support the latest yui3 - I had to include the necessary JS manually and I forgot to add exception for the non-minified stuff. I will fix it somehow soon, thanks!
          Hide
          Petr Škoda added a comment -

          I am working on a bit different fix...

          Show
          Petr Škoda added a comment - I am working on a bit different fix...
          Hide
          Petr Škoda added a comment - - edited

          hmm, I should have read the comment from Sam properly, I came independently to similar solution + some extra filter fix and phpdocs cleanup, thanks!

          To integrators: please note 21 branch does not contain phpdocs fixes intentionally

          Show
          Petr Škoda added a comment - - edited hmm, I should have read the comment from Sam properly, I came independently to similar solution + some extra filter fix and phpdocs cleanup, thanks! To integrators: please note 21 branch does not contain phpdocs fixes intentionally
          Hide
          Tim Hunt added a comment -

          Looks good to me. Thanks for doing a proper fix Petr. +1

          Show
          Tim Hunt added a comment - Looks good to me. Thanks for doing a proper fix Petr. +1
          Hide
          Sam Hemelryk added a comment -

          Thanks Petr, I've integrated the fix now

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Thanks Petr, I've integrated the fix now Cheers Sam
          Hide
          Gerard Caulfield added a comment - - edited

          Looks great.

          Side note: The following bulky scripts never appear to be minified, however this was not part of the test:
          /theme/yui_combo.php?moodle/288/block_navigation/navigation/navigation.js
          /theme/yui_combo.php?moodle/288/calendar/eventmanager/eventmanager.js

          For part 4 I assumed outputrequirements[lib].php was meant.

          Show
          Gerard Caulfield added a comment - - edited Looks great. Side note: The following bulky scripts never appear to be minified, however this was not part of the test: /theme/yui_combo.php?moodle/288/block_navigation/navigation/navigation.js /theme/yui_combo.php?moodle/288/calendar/eventmanager/eventmanager.js For part 4 I assumed outputrequirements [lib] .php was meant.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Whoever decided one week was worth 14 days had really one bad idea. Anyway, the nightmare is over, so thanks for your, once again, amazing contributions. Many, many thanks!

          Now... disconnect, relax and enjoy the next days, yay!

          Closing...ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Whoever decided one week was worth 14 days had really one bad idea. Anyway, the nightmare is over, so thanks for your, once again, amazing contributions. Many, many thanks! Now... disconnect, relax and enjoy the next days, yay! Closing...ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: