diff --git a/library/HTMLPurifier/AttrDef/CSS.php b/library/HTMLPurifier/AttrDef/CSS.php
index ca1b0224..220ec0d0 100644
--- a/library/HTMLPurifier/AttrDef/CSS.php
+++ b/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
{
diff --git a/library/HTMLPurifier/AttrDef/ListStyle.php b/library/HTMLPurifier/AttrDef/ListStyle.php
index a4ea815a..488637b4 100644
--- a/library/HTMLPurifier/AttrDef/ListStyle.php
+++ b/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);
}
diff --git a/tests/HTMLPurifier/AttrDef/CSSTest.php b/tests/HTMLPurifier/AttrDef/CSSTest.php
index 336914e2..96789ebb 100644
--- a/tests/HTMLPurifier/AttrDef/CSSTest.php
+++ b/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;',
diff --git a/tests/HTMLPurifier/AttrDef/ListStyleTest.php b/tests/HTMLPurifier/AttrDef/ListStyleTest.php
index a12080f8..95ef9444 100644
--- a/tests/HTMLPurifier/AttrDef/ListStyleTest.php
+++ b/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');
+
}