Skip to content

Commit

Permalink
Fix multiplication of mixed units to preserve the first unit
Browse files Browse the repository at this point in the history
Match the internal unit handling for math operations to
the Less.js 2.5.3 logic.

The following part of lessjs-2.5.3/less/variables.less fixture
was producing incorrect output:

```
    div-px-1: (@px / @em);
    div-px-2: ((@px / @em) / @cm);
    sub-px-1: (@px - @em);
    sub-cm-1: (@cm - (@px - @em));
    mul-px-1: (@px * @em);
    mul-em-1: (@em * @px);
    mul-em-2: ((@em * @px) * @cm);
    mul-cm-1: (@cm * (@em * @px));
    add-px-1: (@px + @em);
    add-px-2: ((@px + @em) + @cm);
    mul-px-2: ((1 * @px) * @cm);
    mul-px-3: ((@px * 1) * @cm);
```

Without this patch:

```

Change-Id: Ia53cc26232da40e17c8e9829284ca4669c68f4af
--- actual
+++ /less.php/test/Fixtures/lessjs-2.5.3/css/variables.css
@@ -56,13 +56,13 @@
   div-px-1: 10px;
   div-px-2: 1px;
   sub-px-1: 12.6px;
-  sub-cm-1: 9.666625420000001cm;
+  sub-cm-1: 9.666625cm;
-  mul-px-1: 19.6em;
+  mul-px-1: 19.6px;
   mul-em-1: 19.6em;
-  mul-em-2: 196cm;
+  mul-em-2: 196em;
   mul-cm-1: 196cm;
   add-px-1: 15.4px;
-  add-px-2: 393.35323207px;
+  add-px-2: 393.35275591px;
-  mul-px-2: 140cm;
+  mul-px-2: 140px;
-  mul-px-3: 140cm;
+  mul-px-3: 140px;
```

With this patch:

```
--- actual
+++ /less.php/test/Fixtures/lessjs-2.5.3/css/variables.css
@@ -56,13 +56,13 @@
   div-px-1: 10px;
   div-px-2: 1px;
   sub-px-1: 12.6px;
-  sub-cm-1: 9.666625420000001cm;
+  sub-cm-1: 9.666625cm;
   mul-px-1: 19.6px;
   mul-em-1: 19.6em;
   mul-em-2: 196em;
   mul-cm-1: 196cm;
   add-px-1: 15.4px;
-  add-px-2: 393.35323207px;
+  add-px-2: 393.35275591px;
   mul-px-2: 140px;
   mul-px-3: 140px;
 }
```

Bug: T362341
Change-Id: Ia9349fd8d7c3f08962fb5dd3af0d2f57cf78ea97
  • Loading branch information
polishdeveloper authored and Krinkle committed May 12, 2024
1 parent afaa0c0 commit 7346ba1
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 21 deletions.
7 changes: 4 additions & 3 deletions lib/Less/Tree/Dimension.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<?php
/**
* @private
* @see less-2.5.3.js#Dimension.prototype
*/
class Less_Tree_Dimension extends Less_Tree implements Less_Tree_HasValueProperty {

Expand Down Expand Up @@ -76,11 +77,11 @@ public function __toString() {
*/
public function operate( $op, $other ) {
$value = $this->_operate( $op, $this->value, $other->value );
$unit = clone $this->unit;
$unit = $this->unit->clone();

if ( $op === '+' || $op === '-' ) {
if ( !$unit->numerator && !$unit->denominator ) {
$unit = clone $other->unit;
$unit = $other->unit->clone();
if ( $this->unit->backupUnit ) {
$unit->backupUnit = $this->unit->backupUnit;
}
Expand Down Expand Up @@ -141,7 +142,7 @@ public function unify() {

public function convertTo( $conversions ) {
$value = $this->value;
$unit = clone $this->unit;
$unit = $this->unit->clone();

if ( is_string( $conversions ) ) {
$derivedConversions = [];
Expand Down
36 changes: 18 additions & 18 deletions lib/Less/Tree/Unit.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<?php
/**
* @private
* @see less-2.5.3.js#Unit.prototype
*/
class Less_Tree_Unit extends Less_Tree {

Expand All @@ -11,22 +12,28 @@ class Less_Tree_Unit extends Less_Tree {
public function __construct( $numerator = [], $denominator = [], $backupUnit = null ) {
$this->numerator = $numerator;
$this->denominator = $denominator;
$this->backupUnit = $backupUnit;
sort( $this->numerator );
sort( $this->denominator );
$this->backupUnit = $backupUnit ?? $numerator[0] ?? null;
}

public function __clone() {
public function clone() {
// we are recreating a new object to trigger logic from constructor
return new Less_Tree_Unit( $this->numerator, $this->denominator, $this->backupUnit );
}

/**
* @see Less_Tree::genCSS
*/
public function genCSS( $output ) {
if ( $this->numerator ) {
$output->add( $this->numerator[0] );
} elseif ( $this->denominator ) {
$output->add( $this->denominator[0] );
} elseif ( !Less_Parser::$options['strictUnits'] && $this->backupUnit ) {
$strictUnits = Less_Parser::$options['strictUnits'];

if ( count( $this->numerator ) === 1 ) {
$output->add( $this->numerator[0] ); // the ideal situation
} elseif ( !$strictUnits && $this->backupUnit ) {
$output->add( $this->backupUnit );
} elseif ( !$strictUnits && $this->denominator ) {
$output->add( $this->denominator[0] );
}
}

Expand Down Expand Up @@ -58,6 +65,7 @@ public function isLength() {
return (bool)preg_match( '/px|em|%|in|cm|mm|pc|pt|ex/', $css );
}

// TODO: Remove unused method
public function isAngle() {
return isset( Less_Tree_UnitConversions::$angle[$this->toCSS()] );
}
Expand Down Expand Up @@ -92,21 +100,17 @@ public function usedUnits() {
return $result;
}

/**
* @see less-2.5.3.js#Unit.prototype.cancel
*/
public function cancel() {
$counter = [];
$backup = null;

foreach ( $this->numerator as $atomicUnit ) {
if ( !$backup ) {
$backup = $atomicUnit;
}
$counter[$atomicUnit] = ( $counter[$atomicUnit] ?? 0 ) + 1;
}

foreach ( $this->denominator as $atomicUnit ) {
if ( !$backup ) {
$backup = $atomicUnit;
}
$counter[$atomicUnit] = ( $counter[$atomicUnit] ?? 0 ) - 1;
}

Expand All @@ -125,10 +129,6 @@ public function cancel() {
}
}

if ( !$this->numerator && !$this->denominator && $backup ) {
$this->backupUnit = $backup;
}

sort( $this->numerator );
sort( $this->denominator );
}
Expand Down

0 comments on commit 7346ba1

Please sign in to comment.