Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor specificity #59

Merged
merged 5 commits into from
Jul 25, 2014
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 17 additions & 44 deletions CssToInlineStyles.php
Original file line number Diff line number Diff line change
Expand Up @@ -146,39 +146,6 @@ private function buildXPathQuery($selector)
return str_replace('] *', ']//*', $xPath);
}

/**
* Calculate the specifity for the CSS-selector
*
* @return int
* @param string $selector The selector to calculate the specifity for.
*/
private function calculateCSSSpecifity($selector)
{
// cleanup selector
$selector = str_replace(array('>', '+'), array(' > ', ' + '), $selector);

// init var
$specifity = 0;

// split the selector into chunks based on spaces
$chunks = explode(' ', $selector);

// loop chunks
foreach ($chunks as $chunk) {
// an ID is important, so give it a high specifity
if(strstr($chunk, '#') !== false) $specifity += 100;

// classes are more important than a tag, but less important then an ID
elseif(strstr($chunk, '.')) $specifity += 10;

// anything else isn't that important
else $specifity += 1;
}

// return
return $specifity;
}


/**
* Cleanup the generated HTML
Expand Down Expand Up @@ -547,10 +514,11 @@ private function processCSS()
$cssProperties
);

// calculate specifity
$ruleSet['specifity'] = $this->calculateCSSSpecifity(
$selector
) + $i;
// calculate specificity
$ruleSet['specificity'] = Specificity::fromSelector($selector);

// remember the order in which the rules appear
$ruleSet['order'] = $i;

// add into global rules
$this->cssRules[] = $ruleSet;
Expand All @@ -560,9 +528,9 @@ private function processCSS()
$i++;
}

// sort based on specifity
// sort based on specificity
if (!empty($this->cssRules)) {
usort($this->cssRules, array(__CLASS__, 'sortOnSpecifity'));
usort($this->cssRules, array(__CLASS__, 'sortOnSpecificity'));
}
}

Expand Down Expand Up @@ -699,17 +667,22 @@ private function stripOriginalStyleTags($html)
}

/**
* Sort an array on the specifity element
* Sort an array on the specificity element
*
* @return int
* @param array $e1 The first element.
* @param array $e2 The second element.
*/
private static function sortOnSpecifity($e1, $e2)
private static function sortOnSpecificity($e1, $e2)
{
if ($e1['specifity'] == $e2['specifity']) {
return 0;
// Compare the specificity
$value = $e1['specificity']->compare($e2['specificity']);

// if the specificity is the same, use the order in which the element appeared
if ($value === 0) {
$value = $e1['order'] - $e2['order'];
}
return ($e1['specifity'] < $e2['specifity']) ? -1 : 1;

return $value;
}
}
112 changes: 112 additions & 0 deletions Specificity.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
<?php
namespace TijsVerkoyen\CssToInlineStyles;

/**
* CSS to Inline Styles Specificity class.
*
* Compare specificity based on the CSS3 spec.
*
* @see http://www.w3.org/TR/selectors/#specificity
*
*/
class Specificity{

/**
* The number of ID selectors in the selector
*
* @var int
*/
private $a;

/**
*
* The number of class selectors, attributes selectors, and pseudo-classes in the selector
*
* @var int
*/
private $b;

/**
* The number of type selectors and pseudo-elements in the selector
*
* @var int
*/
private $c;

/**
* @param int $a The number of ID selectors in the selector
* @param int $b The number of class selectors, attributes selectors, and pseudo-classes in the selector
* @param int $c The number of type selectors and pseudo-elements in the selector
*/
public function __construct($a=0, $b=0, $c=0)
{
$this->a = $a;
$this->b = $b;
$this->c = $c;
}

/**
* Increase the current specificity by adding the three values
*
* @param int $a The number of ID selectors in the selector
* @param int $b The number of class selectors, attributes selectors, and pseudo-classes in the selector
* @param int $c The number of type selectors and pseudo-elements in the selector
*/
public function increase($a, $b, $c)
{
$this->a += $a;
$this->b += $b;
$this->c += $c;
}

/**
* Calculate the specificity based on a CSS Selector string
*
* @param string $selector
* @return static
*/
public static function fromSelector($selector)
{
// cleanup selector
$selector = str_replace(array('>', '+'), array(' > ', ' + '), $selector);

// create a new instance
$specificity = new static;

// split the selector into chunks based on spaces
$chunks = explode(' ', $selector);

// loop chunks
foreach ($chunks as $chunk) {
// an ID is important, so give it a high specificity
if(strstr($chunk, '#') !== false) $specificity->increase(1,0,0);

// classes are more important than a tag, but less important then an ID
elseif(strstr($chunk, '.')) $specificity->increase(0,1,0);

// anything else isn't that important
else $specificity->increase(0,0,1);
}

// return
return $specificity;

}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tjhink this logic does not work properly for #id.class1.class2::hover for instance. And it also break for [data-type="foo bar"].

In the first case, it will set it to 1, 0, 0 instead of 1, 2, 1. In the second case, it will set it to 0, 0, 2 instead of 0, 1, 0

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, but that was also the case earlier.
We could bring in the Selector Parse from the CssSelector component, if that's not a bit overkill?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we can only use the compare method from the Specificity class when Symfony 2.6 is released, so we would have to use the value (shouldn't be too big a problem, but neither are perfect)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I already thought about getting the specificity from the Symfony parser. I haven't yet tried how much refactoring it would involve in the library (it would not be doable when using CssSelector::toXPath static method as this one does not give you the parsed selector at all btw).

and bumping the dependency to 2.6+ would also be an issue, because it would make the library unusable in Symfony <2.6 projects. If we use the value, we would face the same bug when you have big selectors

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

however, the first fix is indeed to separate the specificity and the order handling for the comparison, which is making the bug much more likely to happen.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe counting the occurrences of the # and . in a string, as starters?

    foreach ($chunks as $chunk) {
        $first = substr($chunk, 0, 1);
        $a = substr_count($chunk, '#');
        $b = substr_count($chunk, '.');
        $c = ($first !== '.' && $first !== '#') ? 1 : 0;
        $specificity->increase($a,$b,$c);
    }

I'm not very good at regexes, otherwise that might be an idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And perhaps better just merge this PR and then create a new issue to focus on getting the actual correct specificity? What do you think @tijsverkoyen ?


/**
* Returns <0 when $specificity is greater, 0 when equal, >0 when smaller
*
* @param Specificity $specificity
* @return int
*/
public function compare(Specificity $specificity)
{
if ($this->a !== $specificity->a) {
return $this->a - $specificity->a;
} elseif ($this->b !== $specificity->b) {
return $this->b - $specificity->b;
} else {
return $this->c - $specificity->c;
}
}
}