[1.4.0] Refactored ListStyle, since list-style-image was sort of tacked on and didn't really conform with the standard. Implementation is still a little flaky but conforms with W3C's validation service.

git-svn-id: http://htmlpurifier.org/svnroot/htmlpurifier/trunk@642 48356398-32a2-884e-a903-53898d9a118a

library/HTMLPurifier/AttrDef/CSS.php

@@ -8,6 +8,11 @@ require_once 'HTMLPurifier/CSSDefinition.php';
  * @note We don't implement the whole CSS specification, so it might be
  *       difficult to reuse this component in the context of validating
  *       actual stylesheet declarations.
+ * @note If we were really serious about validating the CSS, we would
+ *       tokenize the styles and then parse the tokens. Obviously, we
+ *       are not doing that. Doing that could seriously harm performance,
+ *       but would make these components a lot more viable for a CSS
+ *       filtering solution.
  */
 class HTMLPurifier_AttrDef_CSS extends HTMLPurifier_AttrDef
 {

library/HTMLPurifier/AttrDef/ListStyle.php

@@ -4,8 +4,7 @@ require_once 'HTMLPurifier/AttrDef.php';
 
 /**
  * Validates shorthand CSS property list-style.
- * @note This currently does not support list-style-image, as that functionality
- *       is not implemented yet elsewhere.
+ * @warning Does not support url tokens that have internal spaces.
  */
 class HTMLPurifier_AttrDef_ListStyle extends HTMLPurifier_AttrDef
 {
@@ -29,57 +28,49 @@ class HTMLPurifier_AttrDef_ListStyle extends HTMLPurifier_AttrDef
         $string = $this->parseCDATA($string);
         if ($string === '') return false;
         
+        // assumes URI doesn't have spaces in it
         $bits = explode(' ', strtolower($string)); // bits to process
         
-        $caught_type = false;
-        $caught_position = false;
-        $caught_image = false;
-        $caught_none = false; // as in keyword none, which is in all of them
+        $caught = array();
+        $caught['type']     = false;
+        $caught['position'] = false;
+        $caught['image']    = false;
         
-        $ret = '';
+        $i = 0; // number of catches
         
         foreach ($bits as $bit) {
-            if ($caught_none && ($caught_type || $caught_position || $caught_image)) break;
-            if ($caught_type && $caught_position && $caught_image) break;
-            
+            if ($i >= 3) return; // optimization bit
             if ($bit === '') continue;
-            
-            if ($bit === 'none') {
-                if ($caught_none) continue;
-                $caught_none = true;
-                $ret .= 'none ';
-                continue;
-            }
-            
-            // if we add anymore, roll it into a loop
-            
-            $r = $this->info['list-style-type']->validate($bit, $config, $context);
-            if ($r !== false) {
-                if ($caught_type) continue;
-                $caught_type = true;
-                $ret .= $r . ' ';
-                continue;
-            }
-            
-            $r = $this->info['list-style-position']->validate($bit, $config, $context);
-            if ($r !== false) {
-                if ($caught_position) continue;
-                $caught_position = true;
-                $ret .= $r . ' ';
-                continue;
-            }
-            
-            $r = $this->info['list-style-image']->validate($bit, $config, $context);
-            if ($r !== false) {
-                if ($caught_image) continue;
-                $caught_image = true;
-                $ret .= $r . ' ';
-                continue;
+            foreach ($caught as $key => $status) {
+                if ($status !== false) continue;
+                if ($key == 'type' && $bit == 'none') {
+                    // there's no none for image, since you simply
+                    // omit it if you don't want to use it.
+                    $r = 'none';
+                } else {
+                    $r = $this->info['list-style-' . $key]->validate($bit, $config, $context);
+                }
+                if ($r === false) continue;
+                $caught[$key] = $r;
+                $i++;
             }
         }
         
-        $ret = rtrim($ret);
-        return $ret ? $ret : false;
+        if (!$i) return false;
+        
+        $ret = array();
+        
+        // construct type
+        if ($caught['type']) $ret[] = $caught['type'];
+        
+        // construct image
+        if ($caught['image']) $ret[] = $caught['image'];
+        
+        // construct position
+        if ($caught['position']) $ret[] = $caught['position'];
+        
+        if (empty($ret)) return false;
+        return implode(' ', $ret);
         
     }
     

tests/HTMLPurifier/AttrDef/CSSTest.php

@@ -73,7 +73,7 @@ class HTMLPurifier_AttrDef_CSSTest extends HTMLPurifier_AttrDefHarness
         $this->assertDef('vertical-align:50%;');
         $this->assertDef('table-layout:fixed;');
         $this->assertDef('list-style-image:url(nice.jpg);');
-        $this->assertDef('list-style:inside url(nice.jpg) disc;');
+        $this->assertDef('list-style:disc url(nice.jpg) inside;');
         
         // duplicates
         $this->assertDef('text-align:right;text-align:left;',

tests/HTMLPurifier/AttrDef/ListStyleTest.php

@@ -15,9 +15,20 @@ class HTMLPurifier_AttrDef_ListStyleTest extends HTMLPurifier_AttrDefHarness
         $this->assertDef('circle outside');
         $this->assertDef('inside');
         $this->assertDef('none');
+        $this->assertDef('url(foo.gif)');
+        $this->assertDef('circle url(foo.gif) inside');
         
+        // invalid values
         $this->assertDef('outside inside', 'outside');
+        
+        // ordering
+        $this->assertDef('url(foo.gif) none', 'none url(foo.gif)');
         $this->assertDef('circle lower-alpha', 'circle');
+        // the spec is ambiguous about what happens in these
+        // cases, so we're going off the W3C CSS validator
+        $this->assertDef('disc none', 'disc');
+        $this->assertDef('none disc', 'none');
+        
         
     }