Merge pull request #204 from Idrinth/master

sabberworm · web-flow · commit cd1ff1118394 · 2020-11-18T13:10:42.000+01:00
Fixes #203
diff --git 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
@@ -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,18 @@ public function getLineNo() {
 		return $this->iLineNo;
 	}
 
+	/**
+	 * @return int
+	 */
+	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/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
@@ -78,6 +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);
+			}
+		}
+		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);
 			}
 		}
 
@@ -102,6 +112,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;
 	}
 
diff --git 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->assertEquals(
+			array(
+				'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())
+		);
+	}
+
 }

Original file line number	Diff line number	Diff line change
`@@ -44,6 +44,10 @@ public function currentLine() {`
`44`	`44`	`return $this->iLineNo;`
`45`	`45`	`}`
`46`	`46`
	`47`	`+ public function currentColumn() {`
	`48`	`+ return $this->iCurrentPosition;`
	`49`	`+ }`
	`50`	`+`
`47`	`51`	`public function getSettings() {`
`48`	`52`	`return $this->oParserSettings;`
`49`	`53`	`}`
Original file line number	Diff line number	Diff line change
`@@ -189,7 +189,7 @@ public function expandBorderShorthand() {`
`189`	`189`	`$sNewRuleName = $sBorderRule . "-style";`
`190`	`190`	`}`
`191`	`191`	`}`
`192`		`- $oNewRule = new Rule($sNewRuleName, $this->iLineNo);`
	`192`	`+ $oNewRule = new Rule($sNewRuleName, $oRule->getLineNo(), $oRule->getColNo());`
`193`	`193`	`$oNewRule->setIsImportant($oRule->getIsImportant());`
`194`	`194`	`$oNewRule->addValue(array($mNewValue));`
`195`	`195`	`$this->addRule($oNewRule);`
`@@ -245,7 +245,7 @@ public function expandDimensionsShorthand() {`
`245`	`245`	`break;`
`246`	`246`	`}`
`247`	`247`	`foreach (array('top', 'right', 'bottom', 'left') as $sPosition) {`
`248`		`- $oNewRule = new Rule(sprintf($sExpanded, $sPosition), $this->iLineNo);`
	`248`	`+ $oNewRule = new Rule(sprintf($sExpanded, $sPosition), $oRule->getLineNo(), $oRule->getColNo());`
`249`	`249`	`$oNewRule->setIsImportant($oRule->getIsImportant());`
`250`	`250`	`$oNewRule->addValue(${$sPosition});`
`251`	`251`	`$this->addRule($oNewRule);`
`@@ -310,7 +310,7 @@ public function expandFontShorthand() {`
`310`	`310`	`}`
`311`	`311`	`}`
`312`	`312`	`foreach ($aFontProperties as $sProperty => $mValue) {`
`313`		`- $oNewRule = new Rule($sProperty, $this->iLineNo);`
	`313`	`+ $oNewRule = new Rule($sProperty, $oRule->getLineNo(), $oRule->getColNo());`
`314`	`314`	`$oNewRule->addValue($mValue);`
`315`	`315`	`$oNewRule->setIsImportant($oRule->getIsImportant());`
`316`	`316`	`$this->addRule($oNewRule);`
`@@ -344,7 +344,7 @@ public function expandBackgroundShorthand() {`
`344`	`344`	`}`
`345`	`345`	`if (count($aValues) == 1 && $aValues[0] == 'inherit') {`
`346`	`346`	`foreach ($aBgProperties as $sProperty => $mValue) {`
`347`		`- $oNewRule = new Rule($sProperty, $this->iLineNo);`
	`347`	`+ $oNewRule = new Rule($sProperty, $oRule->getLineNo(), $oRule->getColNo());`
`348`	`348`	`$oNewRule->addValue('inherit');`
`349`	`349`	`$oNewRule->setIsImportant($oRule->getIsImportant());`
`350`	`350`	`$this->addRule($oNewRule);`
`@@ -378,7 +378,7 @@ public function expandBackgroundShorthand() {`
`378`	`378`	`}`
`379`	`379`	`}`
`380`	`380`	`foreach ($aBgProperties as $sProperty => $mValue) {`
`381`		`- $oNewRule = new Rule($sProperty, $this->iLineNo);`
	`381`	`+ $oNewRule = new Rule($sProperty, $oRule->getLineNo(), $oRule->getColNo());`
`382`	`382`	`$oNewRule->setIsImportant($oRule->getIsImportant());`
`383`	`383`	`$oNewRule->addValue($mValue);`
`384`	`384`	`$this->addRule($oNewRule);`
`@@ -414,7 +414,7 @@ public function expandListStyleShorthand() {`
`414`	`414`	`}`
`415`	`415`	`if (count($aValues) == 1 && $aValues[0] == 'inherit') {`
`416`	`416`	`foreach ($aListProperties as $sProperty => $mValue) {`
`417`		`- $oNewRule = new Rule($sProperty, $this->iLineNo);`
	`417`	`+ $oNewRule = new Rule($sProperty, $oRule->getLineNo(), $oRule->getColNo());`
`418`	`418`	`$oNewRule->addValue('inherit');`
`419`	`419`	`$oNewRule->setIsImportant($oRule->getIsImportant());`
`420`	`420`	`$this->addRule($oNewRule);`
`@@ -435,7 +435,7 @@ public function expandListStyleShorthand() {`
`435`	`435`	`}`
`436`	`436`	`}`
`437`	`437`	`foreach ($aListProperties as $sProperty => $mValue) {`
`438`		`- $oNewRule = new Rule($sProperty, $this->iLineNo);`
	`438`	`+ $oNewRule = new Rule($sProperty, $oRule->getLineNo(), $oRule->getColNo());`
`439`	`439`	`$oNewRule->setIsImportant($oRule->getIsImportant());`
`440`	`440`	`$oNewRule->addValue($mValue);`
`441`	`441`	`$this->addRule($oNewRule);`
`@@ -465,7 +465,7 @@ public function createShorthandProperties(array $aProperties, $sShorthand) {`
`465`	`465`	`}`
`466`	`466`	`}`
`467`	`467`	`if (count($aNewValues)) {`
`468`		`- $oNewRule = new Rule($sShorthand, $this->iLineNo);`
	`468`	`+ $oNewRule = new Rule($sShorthand, $oRule->getLineNo(), $oRule->getColNo());`
`469`	`469`	`foreach ($aNewValues as $mValue) {`
`470`	`470`	`$oNewRule->addValue($mValue);`
`471`	`471`	`}`
`@@ -538,7 +538,7 @@ public function createDimensionsShorthand() {`
`538`	`538`	`}`
`539`	`539`	`$aValues[$sPosition] = $aRuleValues;`
`540`	`540`	`}`
`541`		`- $oNewRule = new Rule($sProperty, $this->iLineNo);`
	`541`	`+ $oNewRule = new Rule($sProperty, $oRule->getLineNo(), $oRule->getColNo());`
`542`	`542`	`if ((string) $aValues['left'][0] == (string) $aValues['right'][0]) {`
`543`	`543`	`if ((string) $aValues['top'][0] == (string) $aValues['bottom'][0]) {`
`544`	`544`	`if ((string) $aValues['top'][0] == (string) $aValues['left'][0]) {`
`@@ -583,7 +583,9 @@ public function createFontShorthand() {`
`583`	`583`	`if (!isset($aRules['font-size']) \|\| !isset($aRules['font-family'])) {`
`584`	`584`	`return;`
`585`	`585`	`}`
`586`		`- $oNewRule = new Rule('font', $this->iLineNo);`
	`586`	`+ $oOldRule = isset($aRules['font-size']) ? $aRules['font-size'] : $aRules['font-family'];`
	`587`	`+ $oNewRule = new Rule('font', $oOldRule->getLineNo(), $oOldRule->getColNo());`
	`588`	`+ unset($oOldRule);`
`587`	`589`	`foreach (array('font-style', 'font-variant', 'font-weight') as $sProperty) {`
`588`	`590`	`if (isset($aRules[$sProperty])) {`
`589`	`591`	`$oRule = $aRules[$sProperty];`
Original file line number	Diff line number	Diff line change
`@@ -78,6 +78,16 @@ public function addRule(Rule $oRule, Rule $oSibling = null) {`
`78`	`78`	`$iSiblingPos = array_search($oSibling, $this->aRules[$sRule], true);`
`79`	`79`	`if ($iSiblingPos !== false) {`
`80`	`80`	`$iPosition = $iSiblingPos;`
	`81`	`+ $oRule->setPosition($oSibling->getLineNo(), $oSibling->getColNo() - 1);`
	`82`	`+ }`
	`83`	`+ }`
	`84`	`+ if ($oRule->getLineNo() === 0 && $oRule->getColNo() === 0) {`
	`85`	`+ //this node is added manually, give it the next best line`
	`86`	`+ $rules = $this->getRules();`
	`87`	`+ $pos = count($rules);`
	`88`	`+ if ($pos > 0) {`
	`89`	`+ $last = $rules[$pos - 1];`
	`90`	`+ $oRule->setPosition($last->getLineNo() + 1, 0);`
`81`	`91`	`}`
`82`	`92`	`}`
`83`	`93`
`@@ -102,6 +112,12 @@ public function getRules($mRule = null) {`
`102`	`112`	`$aResult = array_merge($aResult, $aRules);`
`103`	`113`	`}`
`104`	`114`	`}`
	`115`	`+ usort($aResult, function (Rule $first, Rule $second) {`
	`116`	`+ if ($first->getLineNo() === $second->getLineNo()) {`
	`117`	`+ return $first->getColNo() - $second->getColNo();`
	`118`	`+ }`
	`119`	`+ return $first->getLineNo() - $second->getLineNo();`
	`120`	`+ });`
`105`	`121`	`return $aResult;`
`106`	`122`	`}`
`107`	`123`