Eliminate JSON Dependency

JSON is used in exactly 2 places in the code and should not be used in either. XMLWriter uses it to cast a float to string. Aside from being silly, this actually causes a problem in Shared/HtmlTest, where one of the results should be 4000 but is instead tested for 3999.9999... The error should have been immaterial (a simple cast will give the correct answer), but the test was wrong, insisting on an exact match for a floating point answer. It should use assertEqualsWithDelta. Next, the reason why JSON got it "wrong" was because the conversions in Shared/Converter use the wrong order of operations - multiplications should be performed before divisions to help avoid rounding problems, but Converter was doing the divisions first. All the conversions there are changed to multiply before dividing. Finally, Shared/Html checks for width units of points or pixels, but should check for inches and centimers as well. Writer/Html outputs tracking properties as a JSON string for some unfathomable reason. It is changed to output each of the three (author, id, and date) as its own discrete property. Tests didn't actually confirm the value of the properties; they do now. Oh, yes, htmlspecialchars is needed for author and id, otherwise the html may wind up broken. Some simple changes are made to TestHelperDOCX to avoid intermittent problems on Windows, and to make the code a little cleaner.
oleibman · Jan 30, 2025 · f82eb47 · f82eb47
1 parent 5271a02
commit f82eb47
Show file tree

Hide file tree

Showing 9 changed files with 86 additions and 35 deletions.
diff --git a/.gitignore b/.gitignore
@@ -24,4 +24,5 @@ phpword.ini
 /nbproject
 /.php_cs.cache
 /.phpunit.result.cache
-/public
+/.phpunit.cache
+/public
diff --git a/composer.json b/composer.json
@@ -109,7 +109,6 @@
         "php": "^7.1|^8.0",
         "ext-dom": "*",
         "ext-gd": "*",
-        "ext-json": "*",
         "ext-xml": "*",
         "ext-zip": "*",
         "phpoffice/math": "^0.2"

diff --git a/src/PhpWord/Shared/Converter.php b/src/PhpWord/Shared/Converter.php
@@ -40,7 +40,7 @@ class Converter
      */
     public static function cmToTwip($centimeter = 1)
     {
-        return $centimeter / self::INCH_TO_CM * self::INCH_TO_TWIP;
+        return $centimeter * self::INCH_TO_TWIP / self::INCH_TO_CM;
     }
 
     /**
@@ -64,7 +64,7 @@ public static function cmToInch($centimeter = 1)
      */
     public static function cmToPixel($centimeter = 1)
     {
-        return $centimeter / self::INCH_TO_CM * self::INCH_TO_PIXEL;
+        return $centimeter * self::INCH_TO_PIXEL / self::INCH_TO_CM;
     }
 
     /**
@@ -76,7 +76,7 @@ public static function cmToPixel($centimeter = 1)
      */
     public static function cmToPoint($centimeter = 1)
     {
-        return $centimeter / self::INCH_TO_CM * self::INCH_TO_POINT;
+        return $centimeter * self::INCH_TO_POINT / self::INCH_TO_CM;
     }
 
     /**
@@ -88,7 +88,7 @@ public static function cmToPoint($centimeter = 1)
      */
     public static function cmToEmu($centimeter = 1)
     {
-        return round($centimeter / self::INCH_TO_CM * self::INCH_TO_PIXEL * self::PIXEL_TO_EMU);
+        return round($centimeter * self::INCH_TO_PIXEL * self::PIXEL_TO_EMU / self::INCH_TO_CM);
     }
 
     /**
@@ -160,7 +160,7 @@ public static function inchToEmu($inch = 1)
      */
     public static function pixelToTwip($pixel = 1)
     {
-        return $pixel / self::INCH_TO_PIXEL * self::INCH_TO_TWIP;
+        return $pixel * self::INCH_TO_TWIP / self::INCH_TO_PIXEL;
     }
 
     /**
@@ -172,7 +172,7 @@ public static function pixelToTwip($pixel = 1)
      */
     public static function pixelToCm($pixel = 1)
     {
-        return $pixel / self::INCH_TO_PIXEL * self::INCH_TO_CM;
+        return $pixel * self::INCH_TO_CM / self::INCH_TO_PIXEL;
     }
 
     /**
@@ -184,7 +184,7 @@ public static function pixelToCm($pixel = 1)
      */
     public static function pixelToPoint($pixel = 1)
     {
-        return $pixel / self::INCH_TO_PIXEL * self::INCH_TO_POINT;
+        return $pixel * self::INCH_TO_POINT / self::INCH_TO_PIXEL;
     }
 
     /**
@@ -208,7 +208,7 @@ public static function pixelToEmu($pixel = 1)
      */
     public static function pointToTwip($point = 1)
     {
-        return $point / self::INCH_TO_POINT * self::INCH_TO_TWIP;
+        return $point * self::INCH_TO_TWIP / self::INCH_TO_POINT;
     }
 
     /**
@@ -220,7 +220,7 @@ public static function pointToTwip($point = 1)
      */
     public static function pointToPixel($point = 1)
     {
-        return $point / self::INCH_TO_POINT * self::INCH_TO_PIXEL;
+        return $point * self::INCH_TO_PIXEL / self::INCH_TO_POINT;
     }
 
     /**
@@ -232,7 +232,7 @@ public static function pointToPixel($point = 1)
      */
     public static function pointToEmu($point = 1)
     {
-        return round($point / self::INCH_TO_POINT * self::INCH_TO_PIXEL * self::PIXEL_TO_EMU);
+        return round($point * self::INCH_TO_PIXEL * self::PIXEL_TO_EMU / self::INCH_TO_POINT);
     }
 
     /**
@@ -244,7 +244,7 @@ public static function pointToEmu($point = 1)
      */
     public static function pointToCm($point = 1)
     {
-        return $point / self::INCH_TO_POINT * self::INCH_TO_CM;
+        return $point * self::INCH_TO_CM / self::INCH_TO_POINT;
     }
 
     /**
@@ -268,7 +268,7 @@ public static function emuToPixel($emu = 1)
      */
     public static function picaToPoint($pica = 1)
     {
-        return $pica / self::INCH_TO_PICA * self::INCH_TO_POINT;
+        return $pica * self::INCH_TO_POINT / self::INCH_TO_PICA;
     }
 
     /**

diff --git a/src/PhpWord/Shared/Html.php b/src/PhpWord/Shared/Html.php
@@ -1477,6 +1477,12 @@ protected static function convertHtmlSize(string $size): float
         if (false !== strpos($size, 'px')) {
             return (float) str_replace('px', '', $size);
         }
+        if (false !== strpos($size, 'cm')) {
+            return Converter::cmToPixel((float) str_replace('cm', '', $size));
+        }
+        if (false !== strpos($size, 'in')) {
+            return Converter::inchToPixel((float) str_replace('in', '', $size));
+        }
 
         return (float) $size;
     }

diff --git a/src/PhpWord/Shared/XMLWriter.php b/src/PhpWord/Shared/XMLWriter.php
@@ -180,7 +180,7 @@ public function writeAttributeIf($condition, $attribute, $value): void
     public function writeAttribute($name, $value)
     {
         if (is_float($value)) {
-            $value = json_encode($value);
+            $value = (string) $value;
         }
 
         return parent::writeAttribute($name, $value ?? '');

diff --git a/src/PhpWord/Writer/HTML/Element/Text.php b/src/PhpWord/Writer/HTML/Element/Text.php
@@ -163,20 +163,22 @@ private function writeTrackChangeOpening()
 
         $content = '';
         if (($changed->getChangeType() == TrackChange::INSERTED)) {
-            $content .= '<ins data-phpword-prop=\'';
+            $content .= '<ins';
         } elseif ($changed->getChangeType() == TrackChange::DELETED) {
-            $content .= '<del data-phpword-prop=\'';
+            $content .= '<del';
         }
-
-        $changedProp = ['changed' => ['author' => $changed->getAuthor(), 'id' => $this->element->getElementId()]];
-        if ($changed->getDate() != null) {
-            $changedProp['changed']['date'] = $changed->getDate()->format('Y-m-d\TH:i:s\Z');
+        $author = htmlspecialchars($changed->getAuthor(), ENT_QUOTES);
+        $content .= " data-phpword-chg-author='$author'";
+        $elementId = htmlspecialchars($this->element->getElementId(), ENT_QUOTES);
+        $content .= " data-phpword-chg-id='$elementId'";
+        $date = $changed->getDate();
+        if ($date !== null) {
+            $dateout = $date->format('Y-m-d\TH:i:s\Z');
+            $content .= " data-phpword-chg-timestamp='$dateout'";
         }
-        $content .= json_encode($changedProp);
-        $content .= '\' ';
-        $content .= 'title="' . $changed->getAuthor();
-        if ($changed->getDate() != null) {
-            $dateUser = $changed->getDate()->format('Y-m-d H:i:s');
+        $content .= ' title="' . $author;
+        if ($date !== null) {
+            $dateUser = $date->format('Y-m-d H:i:s');
             $content .= ' - ' . $dateUser;
         }
         $content .= '">';

diff --git a/tests/PhpWordTests/Shared/HtmlTest.php b/tests/PhpWordTests/Shared/HtmlTest.php
@@ -256,7 +256,8 @@ public function testParseWidth(string $htmlSize, float $docxSize, string $docxUn
         $doc = TestHelperDOCX::getDocument($phpWord, 'Word2007');
         $xpath = '/w:document/w:body/w:tbl/w:tblPr/w:tblW';
         self::assertTrue($doc->elementExists($xpath));
-        self::assertEquals($docxSize, $doc->getElement($xpath)->getAttribute('w:w'));
+        $actual = (float) $doc->getElement($xpath)->getAttribute('w:w');
+        self::assertEqualsWithDelta($docxSize, $actual, 1.0e-12);
         self::assertEquals($docxUnit, $doc->getElement($xpath)->getAttribute('w:type'));
     }
 
@@ -1371,9 +1372,11 @@ public static function providerParseWidth(): array
         return [
             ['auto', 5000, TblWidth::PERCENT],
             ['100%', 5000, TblWidth::PERCENT],
-            ['200pt', 3999.999999999999, TblWidth::TWIP],
+            ['200pt', 4000, TblWidth::TWIP],
             ['300px', 4500, TblWidth::TWIP],
             ['400', 6000, TblWidth::TWIP],
+            ['2in', 2880, TblWidth::TWIP],
+            ['2.54cm', 1440, TblWidth::TWIP],
         ];
     }
 

diff --git a/tests/PhpWordTests/TestHelperDOCX.php b/tests/PhpWordTests/TestHelperDOCX.php
@@ -47,13 +47,14 @@ class TestHelperDOCX
      */
     public static function getDocument(PhpWord $phpWord, $writerName = 'Word2007')
     {
+        $tempdir = self::getTempDirPhpunit();
         self::$file = tempnam(Settings::getTempDir(), 'PhpWord');
         if (false === self::$file) {
             throw new CreateTemporaryFileException();
         }
 
-        if (!is_dir(Settings::getTempDir() . '/PhpWord_Unit_Test/')) {
-            mkdir(Settings::getTempDir() . '/PhpWord_Unit_Test/');
+        if (!is_dir($tempdir)) {
+            mkdir($tempdir);
         }
 
         $xmlWriter = IOFactory::createWriter($phpWord, $writerName);
@@ -62,11 +63,11 @@ public static function getDocument(PhpWord $phpWord, $writerName = 'Word2007')
         $zip = new ZipArchive();
         $res = $zip->open(self::$file);
         if (true === $res) {
-            $zip->extractTo(Settings::getTempDir() . '/PhpWord_Unit_Test/');
+            $zip->extractTo($tempdir);
             $zip->close();
         }
 
-        $doc = new XmlDocument(Settings::getTempDir() . '/PhpWord_Unit_Test/');
+        $doc = new XmlDocument($tempdir);
         if ($writerName === 'ODText') {
             $doc->setDefaultFile('content.xml');
         }
@@ -83,8 +84,9 @@ public static function clear(): void
             unlink(self::$file);
             self::$file = '';
         }
-        if (is_dir(Settings::getTempDir() . '/PhpWord_Unit_Test/')) {
-            self::deleteDir(Settings::getTempDir() . '/PhpWord_Unit_Test/');
+        $tempdir = self::getTempDirPhpunit();
+        if (is_dir($tempdir)) {
+            self::deleteDir($tempdir);
         }
     }
 
@@ -117,4 +119,14 @@ public static function getFile()
     {
         return self::$file;
     }
+
+    /**
+     * Get temporary directory for PhpUnit.
+     *
+     * @return string
+     */
+    private static function getTempDirPhpunit()
+    {
+        return Settings::getTempDir() . '/PhpWord_Unit_Test';
+    }
 }
diff --git a/tests/PhpWordTests/Writer/HTML/ElementTest.php b/tests/PhpWordTests/Writer/HTML/ElementTest.php
@@ -72,13 +72,41 @@ public function testWriteTrackChanges(): void
         $text = $section->addText('my dummy text');
         $text->setChangeInfo(TrackChange::INSERTED, 'author name');
         $text2 = $section->addText('my other text');
-        $text2->setTrackChange(new TrackChange(TrackChange::DELETED, 'another author', new DateTime()));
+        $deleteTime = new DateTime();
+        $deleteAuthor = "Spec O'char";
+        $deleteTrack = new TrackChange(TrackChange::DELETED, $deleteAuthor, $deleteTime);
+        $text2->setTrackChange($deleteTrack);
 
         $dom = Helper::getAsHTML($phpWord);
         $xpath = new DOMXPath($dom);
 
         self::assertEquals(1, $xpath->query('/html/body/div/p[1]/ins')->length);
         self::assertEquals(1, $xpath->query('/html/body/div/p[2]/del')->length);
+        $node = $xpath->query('/html/body/div/p[2]/del');
+        self::assertNotFalse($node);
+        $allAttributes = $node[0]->attributes;
+        self::assertCount(4, $allAttributes);
+        $node = $xpath->query('/html/body/div/p[2]/del');
+        self::assertNotFalse($node);
+
+        $attributes = $node[0]->attributes[0];
+        self::assertSame('data-phpword-chg-author', $attributes->name);
+        self::assertSame($deleteAuthor, $attributes->value);
+
+        $text2Id = $text2->getElementId();
+        $attributes = $node[0]->attributes[1];
+        self::assertSame('data-phpword-chg-id', $attributes->name);
+        self::assertSame($text2Id, $attributes->value);
+
+        $attributes = $node[0]->attributes[2];
+        self::assertSame('data-phpword-chg-timestamp', $attributes->name);
+        self::assertSame($deleteTime->format('Y-m-d\TH:i:s\Z'), $attributes->value);
+
+        $attributes = $node[0]->attributes[3];
+        self::assertSame('title', $attributes->name);
+        $expected = $deleteAuthor . ' - '
+            . $deleteTime->format('Y-m-d H:i:s');
+        self::assertSame($expected, $attributes->value);
     }
 
     /**