Merge pull request #1233 from sjinks/issue-1198

Fix #1198
phalcon · Sep 17, 2013 · 3585034 · 3585034
2 parents cf1d6c2 + 0ace81f
commit 3585034
Show file tree

Hide file tree

Showing 5 changed files with 139 additions and 55 deletions.
diff --git a/ext/assets/manager.c b/ext/assets/manager.c
@@ -455,6 +455,7 @@ PHP_METHOD(Phalcon_Assets_Manager, collection){
  *
  * @param Phalcon\Assets\Collection $collection
  * @param callback $callback
+ * @param string $type
  */
 PHP_METHOD(Phalcon_Assets_Manager, output){
 
@@ -617,7 +618,7 @@ PHP_METHOD(Phalcon_Assets_Manager, output){
 		PHALCON_GET_HVALUE(resource);
 
 		PHALCON_INIT_NVAR(filter_needed);
-		ZVAL_BOOL(filter_needed, 0);
+		ZVAL_FALSE(filter_needed);
 
 		if (!type) {
 			PHALCON_INIT_VAR(type);
@@ -698,18 +699,18 @@ PHP_METHOD(Phalcon_Assets_Manager, output){
 					if (phalcon_file_exists(target_path TSRMLS_CC) == SUCCESS) {
 						if (phalcon_compare_mtime(target_path, source_path TSRMLS_CC)) {
 							PHALCON_INIT_NVAR(filter_needed);
-							ZVAL_BOOL(filter_needed, 1);
+							ZVAL_TRUE(filter_needed);
 						}
 					} else {
 						PHALCON_INIT_NVAR(filter_needed);
-						ZVAL_BOOL(filter_needed, 1);
+						ZVAL_TRUE(filter_needed);
 					}
 				}
 			}
 		}
 
 		/** 
-		 * If there are not filters, just print/buffer the HTML
+		 * If there are no filters, just print/buffer the HTML
 		 */
 		if (Z_TYPE_P(filters) != IS_ARRAY) { 
 
@@ -731,19 +732,17 @@ PHP_METHOD(Phalcon_Assets_Manager, output){
 			/** 
 			 * Prepare the parameters for the callback
 			 */
+			PHALCON_INIT_NVAR(parameters);
+			array_init_size(parameters, 2);
 			if (Z_TYPE_P(attributes) == IS_ARRAY) { 
 				phalcon_array_update_long(&attributes, 0, &prefixed_path, PH_COPY | PH_SEPARATE);
 
-				PHALCON_INIT_NVAR(parameters);
-				array_init_size(parameters, 2);
-				phalcon_array_append(&parameters, attributes, PH_SEPARATE);
-				phalcon_array_append(&parameters, local, PH_SEPARATE);
+				phalcon_array_append(&parameters, attributes, 0);
 			} else {
-				PHALCON_INIT_NVAR(parameters);
-				array_init_size(parameters, 2);
-				phalcon_array_append(&parameters, prefixed_path, PH_SEPARATE);
-				phalcon_array_append(&parameters, local, PH_SEPARATE);
+				phalcon_array_append(&parameters, prefixed_path, 0);
 			}
+
+			phalcon_array_append(&parameters, local, 0);
 
 			/** 
 			 * Call the callback to generate the HTML
@@ -802,31 +801,32 @@ PHP_METHOD(Phalcon_Assets_Manager, output){
 					 */
 					PHALCON_INIT_NVAR(filtered_content);
 					phalcon_call_method_p1(filtered_content, filter, "filter", content);
-
-					/** 
-					 * Update the joined filtered content
-					 */
-					if (zend_is_true(join)) {
-						if (PHALCON_IS_EQUAL(type, type_css)) {
-							if (Z_TYPE_P(filtered_joined_content) == IS_NULL) {
-								PHALCON_INIT_NVAR(filtered_joined_content);
-								PHALCON_CONCAT_VS(filtered_joined_content, filtered_content, "");
-							} else {
-								PHALCON_SCONCAT_VS(filtered_joined_content, filtered_content, "");
-							}
+					PHALCON_CPY_WRT_CTOR(content, filtered_content);
+
+					zend_hash_move_forward_ex(ah1, &hp1);
+				}
+
+				/**
+				 * Update the joined filtered content
+				 */
+				if (zend_is_true(join)) {
+					if (PHALCON_IS_EQUAL(type, type_css)) {
+						if (Z_TYPE_P(filtered_joined_content) == IS_NULL) {
+							PHALCON_INIT_NVAR(filtered_joined_content);
+							PHALCON_CONCAT_VS(filtered_joined_content, content, "");
+						} else {
+							PHALCON_SCONCAT_VS(filtered_joined_content, content, "");
+						}
+					} else {
+						if (Z_TYPE_P(filtered_joined_content) == IS_NULL) {
+							PHALCON_INIT_NVAR(filtered_joined_content);
+							PHALCON_CONCAT_VS(filtered_joined_content, content, ";");
 						} else {
-							if (Z_TYPE_P(filtered_joined_content) == IS_NULL) {
-								PHALCON_INIT_NVAR(filtered_joined_content);
-								PHALCON_CONCAT_VS(filtered_joined_content, filtered_content, ";");
-							} else {
-								PHALCON_SCONCAT_VS(filtered_joined_content, filtered_content, ";");
-							}
+							PHALCON_SCONCAT_VS(filtered_joined_content, content, ";");
 						}
 					}
-
-					zend_hash_move_forward_ex(ah1, &hp1);
 				}
-	
+
 			} else {
 				/** 
 				 * Update the joined filtered content
@@ -875,25 +875,23 @@ PHP_METHOD(Phalcon_Assets_Manager, output){
 			 * Filtered resources are always local
 			 */
 			PHALCON_INIT_NVAR(local);
-			ZVAL_BOOL(local, 1);
+			ZVAL_TRUE(local);
 
 			/** 
 			 * Prepare the parameters for the callback
 			 */
+			PHALCON_INIT_NVAR(parameters);
+			array_init_size(parameters, 2);
 			if (Z_TYPE_P(attributes) == IS_ARRAY) { 
 				phalcon_array_update_long(&attributes, 0, &prefixed_path, PH_COPY | PH_SEPARATE);
 
-				PHALCON_INIT_NVAR(parameters);
-				array_init_size(parameters, 2);
-				phalcon_array_append(&parameters, attributes, PH_SEPARATE);
-				phalcon_array_append(&parameters, local, PH_SEPARATE);
+				phalcon_array_append(&parameters, attributes, 0);
 			} else {
-				PHALCON_INIT_NVAR(parameters);
-				array_init_size(parameters, 2);
-				phalcon_array_append(&parameters, prefixed_path, PH_SEPARATE);
-				phalcon_array_append(&parameters, local, PH_SEPARATE);
+				phalcon_array_append(&parameters, prefixed_path, 0);
 			}
-
+
+			phalcon_array_append(&parameters, local, 0);
+
 			/** 
 			 * Call the callback to generate the HTML
 			 */
@@ -944,25 +942,23 @@ PHP_METHOD(Phalcon_Assets_Manager, output){
 			 * Joined resources are always local
 			 */
 			PHALCON_INIT_NVAR(local);
-			ZVAL_BOOL(local, 1);
+			ZVAL_TRUE(local);
 
 			/** 
 			 * Prepare the parameters for the callback
 			 */
+			PHALCON_INIT_NVAR(parameters);
+			array_init_size(parameters, 2);
 			if (Z_TYPE_P(attributes) == IS_ARRAY) { 
 				phalcon_array_update_long(&attributes, 0, &prefixed_path, PH_COPY | PH_SEPARATE);
 
-				PHALCON_INIT_NVAR(parameters);
-				array_init_size(parameters, 2);
-				phalcon_array_append(&parameters, attributes, PH_SEPARATE);
-				phalcon_array_append(&parameters, local, PH_SEPARATE);
+				phalcon_array_append(&parameters, attributes, 0);
 			} else {
-				PHALCON_INIT_NVAR(parameters);
-				array_init_size(parameters, 2);
-				phalcon_array_append(&parameters, prefixed_path, PH_SEPARATE);
-				phalcon_array_append(&parameters, local, PH_SEPARATE);
+				phalcon_array_append(&parameters, prefixed_path, 0);
 			}
-
+
+			phalcon_array_append(&parameters, local, 0);
+
 			/** 
 			 * Call the callback to generate the HTML
 			 */
@@ -1015,7 +1011,7 @@ PHP_METHOD(Phalcon_Assets_Manager, outputCss){
 	add_next_index_stringl(callback, SL("stylesheetLink"), 1);
 
 	PHALCON_INIT_VAR(type);
-        ZVAL_STRING(type, "css", 1);
+	ZVAL_STRING(type, "css", 1);
 
 	PHALCON_INIT_VAR(output);
 	phalcon_call_method_p3(output, this_ptr, "output", collection, callback, type);

diff --git a/ext/tests/assets/1198.css b/ext/tests/assets/1198.css
@@ -0,0 +1,7 @@
+a {
+	text-decoration: none;
+}
+
+b {
+	font-weight: bold;
+}
diff --git a/ext/tests/issue-1198.phpt b/ext/tests/issue-1198.phpt
@@ -0,0 +1,40 @@
+--TEST--
+Assets Manager doesn't use result of preceding css filter as the source for the next one - https://github.com/phalcon/cphalcon/issues/1198
+--SKIPIF--
+<?php include('skipif.inc'); ?>
+--FILE--
+<?php
+
+class TrimFilter implements Phalcon\Assets\FilterInterface
+{
+	public function filter($s)
+	{
+		return str_replace(array("\n", "\r", " ", "\t"), '', $s);
+	}
+}
+
+class UppercaseFilter implements Phalcon\Assets\FilterInterface
+{
+	public function filter($s)
+	{
+		return strtoupper($s);
+	}
+}
+
+$di = new \Phalcon\DI\FactoryDefault();
+$assets = new \Phalcon\Assets\Manager();
+$assets->useImplicitOutput(false);
+$css = $assets->collection('css');
+$css->setTargetPath(__DIR__ . '/1198.css');
+$css->setTargetUri('/1198.css');
+$css->addCss(__DIR__ . '/assets/1198.css');
+$css->addFilter(new UppercaseFilter());
+$css->addFilter(new TrimFilter());
+$css->join(true);
+$assets->outputCss('css');
+echo file_get_contents(__DIR__ . '/1198.css');
+?>
+--EXPECT--
+A{TEXT-DECORATION:NONE;}B{FONT-WEIGHT:BOLD;}
+--CLEAN--
+<?php unlink(__DIR__ . '/1198.css'); ?>
diff --git a/unit-tests/AssetsTest.php b/unit-tests/AssetsTest.php
@@ -18,6 +18,22 @@
 	+------------------------------------------------------------------------+
 */
 
+class TrimFilter implements \Phalcon\Assets\FilterInterface
+{
+	public function filter($s)
+	{
+		return str_replace(array("\n", "\r", " ", "\t"), '', $s);
+	}
+}
+
+class UppercaseFilter implements \Phalcon\Assets\FilterInterface
+{
+	public function filter($s)
+	{
+		return strtoupper($s);
+	}
+}
+
 class AssetsTest extends PHPUnit_Framework_TestCase
 {
 
@@ -484,4 +500,22 @@ public function testFilterMultiplesSourcesFilterJoin()
 		$this->assertEquals($assets->outputJs('js'), '<script src="/production/combined-3.js" type="text/javascript"></script>' . PHP_EOL);
 	}
 
-}
+	public function testIssue1198()
+	{
+		@unlink(__DIR__ . '/assets/production/1198.css');
+		$di = new \Phalcon\DI\FactoryDefault();
+		$assets = new \Phalcon\Assets\Manager();
+		$assets->useImplicitOutput(false);
+		$css = $assets->collection('css');
+		$css->setTargetPath(__DIR__ . '/assets/production/1198.css');
+		$css->addCss(__DIR__ . '/assets/1198.css');
+		$css->addFilter(new UppercaseFilter());
+		$css->addFilter(new TrimFilter());
+		$css->join(true);
+		$assets->outputCss('css');
+
+		$this->assertEquals(file_get_contents(__DIR__ . '/assets/production/1198.css'), 'A{TEXT-DECORATION:NONE;}B{FONT-WEIGHT:BOLD;}');
+		@unlink(__DIR__ . '/assets/production/1198.css');
+	}
+}
+
diff --git a/unit-tests/assets/1198.css b/unit-tests/assets/1198.css
@@ -0,0 +1,7 @@
+a {
+	text-decoration: none;
+}
+
+b {
+	font-weight: bold;
+}