From d60b996de04eebd1b92144ea8a80a842f022ccdb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20B=C3=BCttner?= Date: Thu, 12 Nov 2020 13:59:23 +0100 Subject: [PATCH 1/5] Fixes #203 Order of rules will now be preserved in most cases when expanding shorthands --- lib/Sabberworm/CSS/Parsing/ParserState.php | 4 ++++ lib/Sabberworm/CSS/Rule/Rule.php | 13 +++++++++-- .../CSS/RuleSet/DeclarationBlock.php | 22 ++++++++++--------- lib/Sabberworm/CSS/RuleSet/RuleSet.php | 6 +++++ 4 files changed, 33 insertions(+), 12 deletions(-) diff --git a/lib/Sabberworm/CSS/Parsing/ParserState.php b/lib/Sabberworm/CSS/Parsing/ParserState.php index daf4919ce..73340700c 100644 --- a/lib/Sabberworm/CSS/Parsing/ParserState.php +++ b/lib/Sabberworm/CSS/Parsing/ParserState.php @@ -44,6 +44,10 @@ public function currentLine() { return $this->iLineNo; } + public function currentColumn() { + return $this->iCurrentPosition; + } + public function getSettings() { return $this->oParserSettings; } diff --git a/lib/Sabberworm/CSS/Rule/Rule.php b/lib/Sabberworm/CSS/Rule/Rule.php index 2afce572a..dc8512e2d 100644 --- a/lib/Sabberworm/CSS/Rule/Rule.php +++ b/lib/Sabberworm/CSS/Rule/Rule.php @@ -19,20 +19,22 @@ class Rule implements Renderable, Commentable { private $bIsImportant; private $aIeHack; protected $iLineNo; + protected $iColNo; protected $aComments; - public function __construct($sRule, $iLineNo = 0) { + public function __construct($sRule, $iLineNo = 0, $iColNo = 0) { $this->sRule = $sRule; $this->mValue = null; $this->bIsImportant = false; $this->aIeHack = array(); $this->iLineNo = $iLineNo; + $this->iColNo = $iColNo; $this->aComments = array(); } public static function parse(ParserState $oParserState) { $aComments = $oParserState->consumeWhiteSpace(); - $oRule = new Rule($oParserState->parseIdentifier(!$oParserState->comes("--")), $oParserState->currentLine()); + $oRule = new Rule($oParserState->parseIdentifier(!$oParserState->comes("--")), $oParserState->currentLine(), $oParserState->currentColumn()); $oRule->setComments($aComments); $oRule->addComments($oParserState->consumeWhiteSpace()); $oParserState->consume(':'); @@ -75,6 +77,13 @@ public function getLineNo() { return $this->iLineNo; } + /** + * @return int + */ + public function getColNo() { + return $this->iColNo; + } + public function setRule($sRule) { $this->sRule = $sRule; } diff --git a/lib/Sabberworm/CSS/RuleSet/DeclarationBlock.php b/lib/Sabberworm/CSS/RuleSet/DeclarationBlock.php index a1142bef0..70092efbd 100644 --- a/lib/Sabberworm/CSS/RuleSet/DeclarationBlock.php +++ b/lib/Sabberworm/CSS/RuleSet/DeclarationBlock.php @@ -189,7 +189,7 @@ public function expandBorderShorthand() { $sNewRuleName = $sBorderRule . "-style"; } } - $oNewRule = new Rule($sNewRuleName, $this->iLineNo); + $oNewRule = new Rule($sNewRuleName, $oRule->getLineNo(), $oRule->getColNo()); $oNewRule->setIsImportant($oRule->getIsImportant()); $oNewRule->addValue(array($mNewValue)); $this->addRule($oNewRule); @@ -245,7 +245,7 @@ public function expandDimensionsShorthand() { break; } foreach (array('top', 'right', 'bottom', 'left') as $sPosition) { - $oNewRule = new Rule(sprintf($sExpanded, $sPosition), $this->iLineNo); + $oNewRule = new Rule(sprintf($sExpanded, $sPosition), $oRule->getLineNo(), $oRule->getColNo()); $oNewRule->setIsImportant($oRule->getIsImportant()); $oNewRule->addValue(${$sPosition}); $this->addRule($oNewRule); @@ -310,7 +310,7 @@ public function expandFontShorthand() { } } foreach ($aFontProperties as $sProperty => $mValue) { - $oNewRule = new Rule($sProperty, $this->iLineNo); + $oNewRule = new Rule($sProperty, $oRule->getLineNo(), $oRule->getColNo()); $oNewRule->addValue($mValue); $oNewRule->setIsImportant($oRule->getIsImportant()); $this->addRule($oNewRule); @@ -344,7 +344,7 @@ public function expandBackgroundShorthand() { } if (count($aValues) == 1 && $aValues[0] == 'inherit') { foreach ($aBgProperties as $sProperty => $mValue) { - $oNewRule = new Rule($sProperty, $this->iLineNo); + $oNewRule = new Rule($sProperty, $oRule->getLineNo(), $oRule->getColNo()); $oNewRule->addValue('inherit'); $oNewRule->setIsImportant($oRule->getIsImportant()); $this->addRule($oNewRule); @@ -378,7 +378,7 @@ public function expandBackgroundShorthand() { } } foreach ($aBgProperties as $sProperty => $mValue) { - $oNewRule = new Rule($sProperty, $this->iLineNo); + $oNewRule = new Rule($sProperty, $oRule->getLineNo(), $oRule->getColNo()); $oNewRule->setIsImportant($oRule->getIsImportant()); $oNewRule->addValue($mValue); $this->addRule($oNewRule); @@ -414,7 +414,7 @@ public function expandListStyleShorthand() { } if (count($aValues) == 1 && $aValues[0] == 'inherit') { foreach ($aListProperties as $sProperty => $mValue) { - $oNewRule = new Rule($sProperty, $this->iLineNo); + $oNewRule = new Rule($sProperty, $oRule->getLineNo(), $oRule->getColNo()); $oNewRule->addValue('inherit'); $oNewRule->setIsImportant($oRule->getIsImportant()); $this->addRule($oNewRule); @@ -435,7 +435,7 @@ public function expandListStyleShorthand() { } } foreach ($aListProperties as $sProperty => $mValue) { - $oNewRule = new Rule($sProperty, $this->iLineNo); + $oNewRule = new Rule($sProperty, $oRule->getLineNo(), $oRule->getColNo()); $oNewRule->setIsImportant($oRule->getIsImportant()); $oNewRule->addValue($mValue); $this->addRule($oNewRule); @@ -465,7 +465,7 @@ public function createShorthandProperties(array $aProperties, $sShorthand) { } } if (count($aNewValues)) { - $oNewRule = new Rule($sShorthand, $this->iLineNo); + $oNewRule = new Rule($sShorthand, $oRule->getLineNo(), $oRule->getColNo()); foreach ($aNewValues as $mValue) { $oNewRule->addValue($mValue); } @@ -538,7 +538,7 @@ public function createDimensionsShorthand() { } $aValues[$sPosition] = $aRuleValues; } - $oNewRule = new Rule($sProperty, $this->iLineNo); + $oNewRule = new Rule($sProperty, $oRule->getLineNo(), $oRule->getColNo()); if ((string) $aValues['left'][0] == (string) $aValues['right'][0]) { if ((string) $aValues['top'][0] == (string) $aValues['bottom'][0]) { if ((string) $aValues['top'][0] == (string) $aValues['left'][0]) { @@ -583,7 +583,9 @@ public function createFontShorthand() { if (!isset($aRules['font-size']) || !isset($aRules['font-family'])) { return; } - $oNewRule = new Rule('font', $this->iLineNo); + $oOldRule = isset($aRules['font-size']) ? $aRules['font-size'] : $aRules['font-family']; + $oNewRule = new Rule('font', $oOldRule->getLineNo(), $oOldRule->getColNo()); + unset($oOldRule); foreach (array('font-style', 'font-variant', 'font-weight') as $sProperty) { if (isset($aRules[$sProperty])) { $oRule = $aRules[$sProperty]; diff --git a/lib/Sabberworm/CSS/RuleSet/RuleSet.php b/lib/Sabberworm/CSS/RuleSet/RuleSet.php index e5d5e4154..470f9d8d6 100644 --- a/lib/Sabberworm/CSS/RuleSet/RuleSet.php +++ b/lib/Sabberworm/CSS/RuleSet/RuleSet.php @@ -102,6 +102,12 @@ public function getRules($mRule = null) { $aResult = array_merge($aResult, $aRules); } } + usort($aResult, function (Rule $first, Rule $second) { + if ($first->getLineNo() === $second->getLineNo()) { + return $first->getColNo() - $second->getColNo(); + } + return $first->getLineNo() - $second->getLineNo(); + }); return $aResult; } From 67c454250c3e6bfe5af7a20748e2614f1ba57eb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20B=C3=BCttner?= Date: Thu, 12 Nov 2020 14:17:41 +0100 Subject: [PATCH 2/5] replacing spaces with tabs to match remaining files --- lib/Sabberworm/CSS/RuleSet/DeclarationBlock.php | 4 ++-- lib/Sabberworm/CSS/RuleSet/RuleSet.php | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/Sabberworm/CSS/RuleSet/DeclarationBlock.php b/lib/Sabberworm/CSS/RuleSet/DeclarationBlock.php index 70092efbd..20fab0d9e 100644 --- a/lib/Sabberworm/CSS/RuleSet/DeclarationBlock.php +++ b/lib/Sabberworm/CSS/RuleSet/DeclarationBlock.php @@ -583,9 +583,9 @@ public function createFontShorthand() { if (!isset($aRules['font-size']) || !isset($aRules['font-family'])) { return; } - $oOldRule = isset($aRules['font-size']) ? $aRules['font-size'] : $aRules['font-family']; + $oOldRule = isset($aRules['font-size']) ? $aRules['font-size'] : $aRules['font-family']; $oNewRule = new Rule('font', $oOldRule->getLineNo(), $oOldRule->getColNo()); - unset($oOldRule); + unset($oOldRule); foreach (array('font-style', 'font-variant', 'font-weight') as $sProperty) { if (isset($aRules[$sProperty])) { $oRule = $aRules[$sProperty]; diff --git a/lib/Sabberworm/CSS/RuleSet/RuleSet.php b/lib/Sabberworm/CSS/RuleSet/RuleSet.php index 470f9d8d6..4d0c2b671 100644 --- a/lib/Sabberworm/CSS/RuleSet/RuleSet.php +++ b/lib/Sabberworm/CSS/RuleSet/RuleSet.php @@ -102,12 +102,12 @@ public function getRules($mRule = null) { $aResult = array_merge($aResult, $aRules); } } - usort($aResult, function (Rule $first, Rule $second) { - if ($first->getLineNo() === $second->getLineNo()) { - return $first->getColNo() - $second->getColNo(); - } - return $first->getLineNo() - $second->getLineNo(); - }); + usort($aResult, function (Rule $first, Rule $second) { + if ($first->getLineNo() === $second->getLineNo()) { + return $first->getColNo() - $second->getColNo(); + } + return $first->getLineNo() - $second->getLineNo(); + }); return $aResult; } From ea6ce858f37dc1a9c4aaf8b9a5a779c062fd52b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20B=C3=BCttner?= Date: Thu, 12 Nov 2020 15:13:42 +0100 Subject: [PATCH 3/5] fixing insert before by manually calculating position --- lib/Sabberworm/CSS/Rule/Rule.php | 5 +++++ lib/Sabberworm/CSS/RuleSet/RuleSet.php | 1 + 2 files changed, 6 insertions(+) diff --git a/lib/Sabberworm/CSS/Rule/Rule.php b/lib/Sabberworm/CSS/Rule/Rule.php index dc8512e2d..306bd35d0 100644 --- a/lib/Sabberworm/CSS/Rule/Rule.php +++ b/lib/Sabberworm/CSS/Rule/Rule.php @@ -84,6 +84,11 @@ public function getColNo() { return $this->iColNo; } + public function setPosition($iLine, $iColumn) { + $this->iColNo = $iColumn; + $this->iLineNo = $iLine; + } + public function setRule($sRule) { $this->sRule = $sRule; } diff --git a/lib/Sabberworm/CSS/RuleSet/RuleSet.php b/lib/Sabberworm/CSS/RuleSet/RuleSet.php index 4d0c2b671..f19ffda2f 100644 --- a/lib/Sabberworm/CSS/RuleSet/RuleSet.php +++ b/lib/Sabberworm/CSS/RuleSet/RuleSet.php @@ -78,6 +78,7 @@ public function addRule(Rule $oRule, Rule $oSibling = null) { $iSiblingPos = array_search($oSibling, $this->aRules[$sRule], true); if ($iSiblingPos !== false) { $iPosition = $iSiblingPos; + $oRule->setPosition($oSibling->getLineNo(), $oSibling->getColNo() - 1); } } From 4fa7b7264517e131152a1ca3a7e6780165022a6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20B=C3=BCttner?= Date: Thu, 12 Nov 2020 15:26:04 +0100 Subject: [PATCH 4/5] adding first draft of a unittest --- .../CSS/RuleSet/DeclarationBlockTest.php | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/Sabberworm/CSS/RuleSet/DeclarationBlockTest.php b/tests/Sabberworm/CSS/RuleSet/DeclarationBlockTest.php index f135684cf..3d9b4ee8c 100644 --- a/tests/Sabberworm/CSS/RuleSet/DeclarationBlockTest.php +++ b/tests/Sabberworm/CSS/RuleSet/DeclarationBlockTest.php @@ -264,4 +264,27 @@ public function testRuleInsertion() { $this->assertSame('.wrapper {left: 16em;left: 10px;text-align: 1;text-align: left;border-bottom-width: 1px;}', $oDoc->render()); } + public function testOrderOfElementsMatchingOriginalOrderAfterExpandingShorthands() + { + $sCss = '.rule{padding:5px;padding-top: 20px}'; + $oParser = new Parser($sCss); + $oDoc = $oParser->parse(); + $aDocs = $oDoc->getAllDeclarationBlocks(); + + $this->assertCount(1, $aDocs); + + $oDeclaration = array_pop($aDocs); + $oDeclaration->expandShorthands(); + + $this->assertEqual( + array( + 'padding-top' => 'padding-top:20px', + 'padding-left' => 'padding-top:5px', + 'padding-rigth' => 'padding-top:5px', + 'padding-bottom' => 'padding-top:5px', + ), + array_map('strval', $oDeclaration->getRulesAssoc()) + ); + } + } From aa323203d84dfaea07e4f60f013594d921917e8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20B=C3=BCttner?= Date: Thu, 12 Nov 2020 16:37:59 +0100 Subject: [PATCH 5/5] fixing failing unit tests --- lib/Sabberworm/CSS/RuleSet/RuleSet.php | 11 ++++++++++- tests/Sabberworm/CSS/RuleSet/DeclarationBlockTest.php | 10 +++++----- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/lib/Sabberworm/CSS/RuleSet/RuleSet.php b/lib/Sabberworm/CSS/RuleSet/RuleSet.php index f19ffda2f..f63a3f168 100644 --- a/lib/Sabberworm/CSS/RuleSet/RuleSet.php +++ b/lib/Sabberworm/CSS/RuleSet/RuleSet.php @@ -78,7 +78,16 @@ public function addRule(Rule $oRule, Rule $oSibling = null) { $iSiblingPos = array_search($oSibling, $this->aRules[$sRule], true); if ($iSiblingPos !== false) { $iPosition = $iSiblingPos; - $oRule->setPosition($oSibling->getLineNo(), $oSibling->getColNo() - 1); + $oRule->setPosition($oSibling->getLineNo(), $oSibling->getColNo() - 1); + } + } + if ($oRule->getLineNo() === 0 && $oRule->getColNo() === 0) { + //this node is added manually, give it the next best line + $rules = $this->getRules(); + $pos = count($rules); + if ($pos > 0) { + $last = $rules[$pos - 1]; + $oRule->setPosition($last->getLineNo() + 1, 0); } } diff --git a/tests/Sabberworm/CSS/RuleSet/DeclarationBlockTest.php b/tests/Sabberworm/CSS/RuleSet/DeclarationBlockTest.php index 3d9b4ee8c..b74fb8a24 100644 --- a/tests/Sabberworm/CSS/RuleSet/DeclarationBlockTest.php +++ b/tests/Sabberworm/CSS/RuleSet/DeclarationBlockTest.php @@ -276,12 +276,12 @@ public function testOrderOfElementsMatchingOriginalOrderAfterExpandingShorthands $oDeclaration = array_pop($aDocs); $oDeclaration->expandShorthands(); - $this->assertEqual( + $this->assertEquals( array( - 'padding-top' => 'padding-top:20px', - 'padding-left' => 'padding-top:5px', - 'padding-rigth' => 'padding-top:5px', - 'padding-bottom' => 'padding-top:5px', + 'padding-top' => 'padding-top: 20px;', + 'padding-right' => 'padding-right: 5px;', + 'padding-bottom' => 'padding-bottom: 5px;', + 'padding-left' => 'padding-left: 5px;', ), array_map('strval', $oDeclaration->getRulesAssoc()) );