Refactor combine_xxx() functions together into combine_files() and use

html functions to generate the resulting elements.  Add phpdoc.

modules/gallery/controllers/combined.php

@@ -22,13 +22,15 @@ class Combined_Controller extends Controller {
    * Return the combined Javascript bundle associated with the given key.
    */
   public function javascript($key) {
+    $key = substr($key, 0, strlen($key) - 3);  // strip off the trailing .js
     return $this->_emit("javascript", $key);
   }
 
   /**
    * Return the combined CSS bundle associated with the given key.
    */
   public function css($key) {
+    $key = substr($key, 0, strlen($key) - 4);  // strip off the trailing .css
     return $this->_emit("css", $key);
   }
 

modules/gallery/libraries/Admin_View.php

@@ -98,9 +98,9 @@ public function __call($function, $args) {
         }
       }
 
-      if ($function == "admin_head") {
-        array_unshift($blocks, $this->combine_css());
-        array_unshift($blocks, $this->combine_script());
+      if ($function == "head") {
+        array_unshift($blocks, $this->combine_files($this->css, "css"));
+        array_unshift($blocks, $this->combine_files($this->css, "javascript"));
       }
 
       if (Session::instance()->get("debug")) {

modules/gallery/libraries/Gallery_View.php

@@ -49,45 +49,6 @@ public function theme_url($path, $absolute_url=false) {
     return $absolute_url ? url::abs_file($arg) : url::file($arg);
   }
 
-  /**
-   * Combine a series of Javascript files into a single one and cache it in the database, then
-   * return a single <script> element to refer to it.
-   */
-  protected function combine_script() {
-    $links = array();
-    $key = "";
-    foreach (array_keys($this->scripts) as $file) {
-      $path = DOCROOT . $file;
-      if (file_exists($path)) {
-        $stats = stat($path);
-        $links[] = $path;
-        // 7 == size, 9 == mtime, see http://php.net/stat
-        $key = "{$key}$file $stats[7] $stats[9],";
-      } else {
-        Kohana::log("alert", "Javascript file missing: " . $file);
-      }
-    }
-
-    $key = md5($key);
-    $cache = Cache::instance();
-    $contents = $cache->get($key);
-    if (empty($contents)) {
-      $contents = "";
-      foreach ($links as $link) {
-        $contents .= file_get_contents($link);
-      }
-      $cache->set($key, $contents, array("javascript"), 30 * 84600);
-      if (function_exists("gzencode")) {
-        $cache->set("{$key}_gz", gzencode($contents, 9, FORCE_GZIP),
-                    array("javascript", "gzip"), 30 * 84600);
-      }
-    }
-
-    // Handcraft the script link because html::script will add a .js extenstion
-    return "<script type=\"text/javascript\" src=\"" . url::site("combined/javascript/$key") .
-      "\"></script>";
-  }
-
   /**
    * Add a css file to the combined css list.
    * @param $file  the relative path to a script from the gallery3 directory
@@ -107,45 +68,59 @@ public function theme_css($file) {
   }
 
   /**
-   * Combine a series of Javascript files into a single one and cache it in the database, then
-   * return a single <script> element to refer to it.
+   * Combine a series of files into a single one and cache it in the database.
    */
-  protected function combine_css() {
+  protected function combine_files($files, $type) {
     $links = array();
-    $key = "";
+    $key = array();
 
     foreach (array_keys($this->css) as $file) {
       $path = DOCROOT . $file;
       if (file_exists($path)) {
         $stats = stat($path);
         $links[] = $path;
         // 7 == size, 9 == mtime, see http://php.net/stat
-        $key = "{$key}$file $stats[7] $stats[9],";
+        $key[] = "$file $stats[7] $stats[9]";
       } else {
-        Kohana::log("alert", "CSS file missing: " . $file);
+        Kohana::log("error", "missing file ($type): $file");
       }
     }
 
-    $key = md5($key);
+    $key = md5(join(" ", $key));
     $cache = Cache::instance();
     $contents = $cache->get($key);
 
     $contents = "";
     if (empty($contents)) {
       $contents = "";
       foreach ($links as $link) {
-        $contents .= $this->process_css($link);
+        if ($type == "css") {
+          $contents .= $this->process_css($link);
+        } else {
+          $contents .= file_get_contents($link);
+        }
       }
-      $cache->set($key, $contents, array("css"), 30 * 84600);
+
+      $cache->set($key, $contents, array($type), 30 * 84600);
       if (function_exists("gzencode")) {
         $cache->set("{$key}_gz", gzencode($contents, 9, FORCE_GZIP),
-                    array("css", "gzip"), 30 * 84600);
+                    array($type, "gzip"), 30 * 84600);
       }
     }
-    return "<link media=\"screen, projection\" rel=\"stylesheet\" type=\"text/css\" href=\"" .
-      url::site("combined/css/$key") . "\" />";
+
+    if ($type == "css") {
+      return html::stylesheet("combined/css/$key", false, true);
+    } else {
+      // Handcraft the script link because html::script will add a .js extenstion
+      return html::script("combined/javascript/$key", true);
+    }
   }
 
+  /**
+   * Convert relative references inside a CSS file to absolute ones so that when it's served from
+   * a new location as part of a combined bundle the references are still correct.
+   * @param string  the path to the css file
+   */
   private function process_css($css_file) {
     static $PATTERN = "#url\(\s*['|\"]{0,1}(.*?)['|\"]{0,1}\s*\)#";
     $docroot_length = strlen(DOCROOT);
@@ -159,8 +134,7 @@ private function process_css($css_file) {
           $search[] = $match[0];
           $replace[] = "url('" . url::abs_file($relative) . "')";
         } else {
-          Kohana::log("alert", sprintf("Missing URL reference '%s' in CSS file '%s' ",
-                                       $match[1], $css_file));
+          Kohana::log("error", "Missing URL reference '{$match[1]}' in CSS file '$css_file'");
         }
       }
       $css = str_replace($search, $replace, $css);
@@ -171,7 +145,6 @@ private function process_css($css_file) {
     if ($imports) {
       $search = $replace = array();
       foreach ($matches as $match) {
-        Kohana::log("error", dirname($css_file) . "/$match[1]");
         $search[] = $match[0];
         $replace[] = $this->process_css(dirname($css_file) . "/$match[1]");
       }

modules/gallery/libraries/Theme_View.php

@@ -216,8 +216,8 @@ public function __call($function, $args) {
       }
 
       if ($function == "head") {
-        array_unshift($blocks, $this->combine_css());
-        array_unshift($blocks, $this->combine_script());
+        array_unshift($blocks, $this->combine_files($this->css, "css"));
+        array_unshift($blocks, $this->combine_files($this->css, "javascript"));
       }
 
       if (Session::instance()->get("debug")) {