From f82eb47f8f6327e8883a6d921ac3b73017d6b78b Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Thu, 30 Jan 2025 15:00:10 -0800 Subject: [PATCH] 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. --- .gitignore | 3 +- composer.json | 1 - src/PhpWord/Shared/Converter.php | 24 +++++++-------- src/PhpWord/Shared/Html.php | 6 ++++ src/PhpWord/Shared/XMLWriter.php | 2 +- src/PhpWord/Writer/HTML/Element/Text.php | 24 ++++++++------- tests/PhpWordTests/Shared/HtmlTest.php | 7 +++-- tests/PhpWordTests/TestHelperDOCX.php | 24 +++++++++++---- .../PhpWordTests/Writer/HTML/ElementTest.php | 30 ++++++++++++++++++- 9 files changed, 86 insertions(+), 35 deletions(-) diff --git a/.gitignore b/.gitignore index 0b9d0608d0..3ebe0dc1a8 100644 --- a/.gitignore +++ b/.gitignore @@ -24,4 +24,5 @@ phpword.ini /nbproject /.php_cs.cache /.phpunit.result.cache -/public \ No newline at end of file +/.phpunit.cache +/public diff --git a/composer.json b/composer.json index 2d06f890b3..9487640f2a 100644 --- 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 index 17d2e1a05d..82e11c0790 100644 --- 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 index 9b57f77adc..d1bf13d7f7 100644 --- 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 index 8dc28e1184..8b267b694c 100644 --- 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 index 312d3a19c2..c6363d1da4 100644 --- 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 .= 'getChangeType() == TrackChange::DELETED) { - $content .= ' ['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 index 142a93e5a5..5131d6ea44 100644 --- 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 index 2a6fbabae0..5ebe799cfc 100644 --- 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 index 3b2580381f..28e785a237 100644 --- 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); } /**