Html Reader Process Titles as Headings Not Paragraphs

oleibman · oleibman · commit 7301f7d80219 · 2023-12-24T15:58:37.000-08:00
Fix PHPOffice#1692. Builds on work started some time ago by @0b10011, to whom primary credit is due. Html Reader does not process the `head` section of the document, and, in particular, does not process its `style` section. It will, however, process inline styles, so 0b10011's model of adding the title as a text run (with styles) will work well once this change is applied. However, that model would not deal with the alternative method of assigning a Title Style, and just adding the title as text. In order to accommodate that, I have removed the declaration of heading font styles in the head section, and now generate them all inline in the body. This has the added benefit of being able to read the doc as html, then saving it as docx, preserving, at least in part, any user-defined font styles. Note that html does have pre-defined title styles, but docx does not. @constip suggests in the original issue that margin top and bottom are being applied too frequently. I believe that was addressed by recently merged PR PHPOffice#2475. It is also suggested that the `*` css selector be dropped in favor of `body`. 2475 added the body selector. I agree that this renders the `*` selector unnecessary, and, as stated in the issue, it can cause problems. This PR drops that selector. It is also suggested that `loadHTML` be used instead of `loadXML`. This is not as easy a change as it seems, because loadHTML uses ISO-8859-1 charset rather than UTF-8, so I will not attempt that change.
diff --git a/docs/changes/1.x/1.3.0.md b/docs/changes/1.x/1.3.0.md
@@ -0,0 +1,15 @@
+# [1.3.0](https://github.com/PHPOffice/PHPWord/tree/1.3.0) (WIP)
+
+[Full Changelog](https://github.com/PHPOffice/PHPWord/compare/1.2.0...1.3.0)
+
+## Enhancements
+
+### Bug fixes
+
+- MsDoc Reader : Correct Font Size Calculation by [@oleibman](https://github.com/oleibman) Issue [#2526](https://github.com/PHPOffice/PHPWord/issues/2526) PR [#2531](https://github.com/PHPOffice/PHPWord/pull/2531)
+- Html Reader : Process Titles as Headings not Paragraphs [@0b10011](https://github.com/0b10011) and [@oleibman](https://github.com/oleibman) Issue [#1692](https://github.com/PHPOffice/PHPWord/issues/1692) PR [#2533](https://github.com/PHPOffice/PHPWord/pull/2533)
+
+### Miscellaneous
+
+
+### BC Breaks
diff --git a/src/PhpWord/Shared/Html.php b/src/PhpWord/Shared/Html.php
@@ -25,6 +25,7 @@
 use PhpOffice\PhpWord\Element\AbstractContainer;
 use PhpOffice\PhpWord\Element\Row;
 use PhpOffice\PhpWord\Element\Table;
+use PhpOffice\PhpWord\Element\TextRun;
 use PhpOffice\PhpWord\Settings;
 use PhpOffice\PhpWord\SimpleType\Jc;
 use PhpOffice\PhpWord\SimpleType\NumberFormat;
@@ -206,12 +207,12 @@ protected static function parseNode($node, $element, $styles = [], $data = []):
         $nodes = [
             // $method        $node   $element    $styles     $data   $argument1      $argument2
             'p' => ['Paragraph',   $node,  $element,   $styles,    null,   null,           null],
-            'h1' => ['Heading',     null,   $element,   $styles,    null,   'Heading1',     null],
-            'h2' => ['Heading',     null,   $element,   $styles,    null,   'Heading2',     null],
-            'h3' => ['Heading',     null,   $element,   $styles,    null,   'Heading3',     null],
-            'h4' => ['Heading',     null,   $element,   $styles,    null,   'Heading4',     null],
-            'h5' => ['Heading',     null,   $element,   $styles,    null,   'Heading5',     null],
-            'h6' => ['Heading',     null,   $element,   $styles,    null,   'Heading6',     null],
+            'h1' => ['Heading',     $node,   $element,   $styles,    null,   'Heading1',     null],
+            'h2' => ['Heading',     $node,   $element,   $styles,    null,   'Heading2',     null],
+            'h3' => ['Heading',     $node,   $element,   $styles,    null,   'Heading3',     null],
+            'h4' => ['Heading',     $node,   $element,   $styles,    null,   'Heading4',     null],
+            'h5' => ['Heading',     $node,   $element,   $styles,    null,   'Heading5',     null],
+            'h6' => ['Heading',     $node,   $element,   $styles,    null,   'Heading6',     null],
             '#text' => ['Text',        $node,  $element,   $styles,    null,   null,           null],
             'strong' => ['Property',    null,   null,       $styles,    null,   'bold',         true],
             'b' => ['Property',    null,   null,       $styles,    null,   'bold',         true],
@@ -337,21 +338,22 @@ protected static function parseInput($node, $element, &$styles): void
     /**
      * Parse heading node.
      *
-     * @param \PhpOffice\PhpWord\Element\AbstractContainer $element
-     * @param array &$styles
-     * @param string $argument1 Name of heading style
-     *
-     * @return \PhpOffice\PhpWord\Element\TextRun
-     *
      * @todo Think of a clever way of defining header styles, now it is only based on the assumption, that
      * Heading1 - Heading6 are already defined somewhere
      */
-    protected static function parseHeading($element, &$styles, $argument1)
+    protected static function parseHeading(DOMNode $node, AbstractContainer $element, array &$styles, string $headingStyle): TextRun
     {
-        $styles['paragraph'] = $argument1;
-        $newElement = $element->addTextRun($styles['paragraph']);
+        self::parseInlineStyle($node, $styles['font']);
+        // Create a TextRun to hold styles and text
+        $styles['paragraph'] = $headingStyle;
+        $textRun = new TextRun($styles['paragraph']);
 
-        return $newElement;
+        // Create a title with level corresponding to number in heading style
+        // (Eg, Heading1 = 1)
+        $element->addTitle($textRun, (int) ltrim($headingStyle, 'Heading'));
+
+        // Return TextRun so children are parsed
+        return $textRun;
     }
 
     /**
diff --git a/src/PhpWord/Writer/HTML/Element/Title.php b/src/PhpWord/Writer/HTML/Element/Title.php
@@ -46,8 +46,14 @@ public function write()
             $writer = new Container($this->parentWriter, $text);
             $text = $writer->write();
         }
+        $css = '';
+        $style = \PhpOffice\PhpWord\Style::getStyle('Heading_' . $this->element->getDepth());
+        if ($style !== null) {
+            $styleWriter = new \PhpOffice\PhpWord\Writer\HTML\Style\Font($style);
+            $css = ' style="' . $styleWriter->write() . '"';
+        }
 
-        $content = "<{$tag}>{$text}</{$tag}>" . PHP_EOL;
+        $content = "<{$tag}{$css}>{$text}</{$tag}>" . PHP_EOL;
 
         return $content;
     }
diff --git a/src/PhpWord/Writer/HTML/Part/Head.php b/src/PhpWord/Writer/HTML/Part/Head.php
@@ -90,17 +90,16 @@ private function writeStyles(): string
             'font-family' => $this->getFontFamily(Settings::getDefaultFontName(), $this->getParentWriter()->getDefaultGenericFont()),
             'font-size' => Settings::getDefaultFontSize() . 'pt',
         ];
-        // Mpdf sometimes needs separate tag for body; doesn't harm others.
-        $bodyarray = $astarray;
 
         $defaultWhiteSpace = $this->getParentWriter()->getDefaultWhiteSpace();
         if ($defaultWhiteSpace) {
             $astarray['white-space'] = $defaultWhiteSpace;
         }
+        $bodyarray = $astarray;
 
         foreach ([
             'body' => $bodyarray,
-            '*' => $astarray,
+            //'*' => $astarray,
             'a.NoteRef' => [
                 'text-decoration' => 'none',
             ],
@@ -137,8 +136,8 @@ private function writeStyles(): string
                         $style = $styleParagraph;
                     } else {
                         $name = '.' . $name;
+                        $css .= "{$name} {" . $styleWriter->write() . '}' . PHP_EOL;
                     }
-                    $css .= "{$name} {" . $styleWriter->write() . '}' . PHP_EOL;
                 }
                 if ($style instanceof Paragraph) {
                     $styleWriter = new ParagraphStyleWriter($style);
diff --git a/tests/PhpWordTests/Shared/HtmlHeadingsTest.php b/tests/PhpWordTests/Shared/HtmlHeadingsTest.php
@@ -0,0 +1,66 @@
+<?php
+/**
+ * This file is part of PHPWord - A pure PHP library for reading and writing
+ * word processing documents.
+ *
+ * PHPWord is free software distributed under the terms of the GNU Lesser
+ * General Public License version 3 as published by the Free Software Foundation.
+ *
+ * For the full copyright and license information, please read the LICENSE
+ * file that was distributed with this source code. For the full list of
+ * contributors, visit https://github.com/PHPOffice/PHPWord/contributors.
+ *
+ * @see         https://github.com/PHPOffice/PHPWord
+ *
+ * @license     http://www.gnu.org/licenses/lgpl.txt LGPL version 3
+ */
+
+namespace PhpOffice\PhpWordTests\Shared;
+
+use PhpOffice\PhpWord\Element\TextRun;
+use PhpOffice\PhpWord\PhpWord;
+use PhpOffice\PhpWord\Settings;
+use PhpOffice\PhpWord\Shared\Html as SharedHtml;
+use PhpOffice\PhpWord\Writer\HTML as HtmlWriter;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * Test class for PhpOffice\PhpWord\Shared\Html.
+ *
+ * @coversDefaultClass \PhpOffice\PhpWord\Shared\Html
+ */
+class HtmlHeadingsTest extends TestCase
+{
+    public function testRoundTripHeadings(): void
+    {
+        Settings::setOutputEscapingEnabled(true);
+        $originalDoc = new PhpWord();
+        $originalDoc->addTitleStyle(1, ['size' => 20]);
+        $section = $originalDoc->addSection();
+        $expectedStrings = [];
+        $section->addTitle('Title 1', 1);
+        $expectedStrings[] = '<h1 style="font-size: 20pt;">Title 1</h1>';
+        for ($i = 2; $i <= 6; ++$i) {
+            $textRun = new TextRun();
+            $textRun->addText('Title ');
+            $textRun->addText("$i", ['italic' => true]);
+            $section->addTitle($textRun, $i);
+            $expectedStrings[] = "<h$i>Title <span style=\"font-style: italic;\">$i</span></h$i>";
+        }
+        $writer = new HtmlWriter($originalDoc);
+        $content = $writer->getContent();
+        foreach ($expectedStrings as $expectedString) {
+            self::assertStringContainsString($expectedString, $content);
+        }
+
+        $newDoc = new PhpWord();
+        $newSection = $newDoc->addSection();
+        SharedHtml::addHtml($newSection, $content, true);
+        $newWriter = new HtmlWriter($newDoc);
+        $newContent = $newWriter->getContent();
+        // Reader transforms Text to TextRun,
+        //  but result is functionally the same.
+        $firstStringAsTextRun = '<h1><span style="font-size: 20pt;">Title 1</span></h1>';
+        self::assertSame($content, str_replace($firstStringAsTextRun, $expectedStrings[0], $newContent));
+    }
+}
diff --git a/tests/PhpWordTests/Writer/HTML/FontTest.php b/tests/PhpWordTests/Writer/HTML/FontTest.php
@@ -84,23 +84,23 @@ public function testFontNames1(): void
         self::assertEquals('style5', Helper::getTextContent($xpath, '/html/body/div/p[6]/span', 'class'));
 
         $style = Helper::getTextContent($xpath, '/html/head/style');
-        $prg = preg_match('/^[*][^\\r\\n]*/m', $style, $matches);
-        self::assertNotFalse($prg);
-        self::assertEquals('* {font-family: \'Courier New\'; font-size: 12pt;}', $matches[0]);
+        $prg = preg_match('/^body[^\\r\\n]*/m', $style, $matches);
+        self::assertSame(1, $prg);
+        self::assertEquals('body {font-family: \'Courier New\'; font-size: 12pt;}', $matches[0]);
         $prg = preg_match('/^[.]style1[^\\r\\n]*/m', $style, $matches);
-        self::assertNotFalse($prg);
+        self::assertSame(1, $prg);
         self::assertEquals('.style1 {font-family: \'Tahoma\'; font-size: 10pt; color: #1B2232; font-weight: bold;}', $matches[0]);
         $prg = preg_match('/^[.]style2[^\\r\\n]*/m', $style, $matches);
-        self::assertNotFalse($prg);
+        self::assertSame(1, $prg);
         self::assertEquals('.style2 {font-family: \'Arial\'; font-size: 10pt;}', $matches[0]);
         $prg = preg_match('/^[.]style3[^\\r\\n]*/m', $style, $matches);
-        self::assertNotFalse($prg);
+        self::assertSame(1, $prg);
         self::assertEquals('.style3 {font-family: \'hack attempt&#039;}; display:none\'; font-size: 10pt;}', $matches[0]);
         $prg = preg_match('/^[.]style4[^\\r\\n]*/m', $style, $matches);
-        self::assertNotFalse($prg);
+        self::assertSame(1, $prg);
         self::assertEquals('.style4 {font-family: \'padmaa 1.1\'; font-size: 10pt; font-weight: bold;}', $matches[0]);
         $prg = preg_match('/^[.]style5[^\\r\\n]*/m', $style, $matches);
-        self::assertNotFalse($prg);
+        self::assertSame(1, $prg);
         self::assertEquals('.style5 {font-family: \'MingLiU-ExtB\'; font-size: 10pt; font-weight: bold;}', $matches[0]);
     }
 
@@ -134,20 +134,20 @@ public function testFontNames2(): void
         self::assertEquals('style4', Helper::getTextContent($xpath, '/html/body/div/p[5]/span', 'class'));
 
         $style = Helper::getTextContent($xpath, '/html/head/style');
-        $prg = preg_match('/^[*][^\\r\\n]*/m', $style, $matches);
-        self::assertNotFalse($prg);
-        self::assertEquals('* {font-family: \'Courier New\'; font-size: 12pt;}', $matches[0]);
+        $prg = preg_match('/^body[^\\r\\n]*/m', $style, $matches);
+        self::assertSame(1, $prg);
+        self::assertEquals('body {font-family: \'Courier New\'; font-size: 12pt;}', $matches[0]);
         $prg = preg_match('/^[.]style1[^\\r\\n]*/m', $style, $matches);
-        self::assertNotFalse($prg);
+        self::assertSame(1, $prg);
         self::assertEquals('.style1 {font-family: \'Tahoma\'; font-size: 10pt; color: #1B2232; font-weight: bold;}', $matches[0]);
         $prg = preg_match('/^[.]style2[^\\r\\n]*/m', $style, $matches);
-        self::assertNotFalse($prg);
+        self::assertSame(1, $prg);
         self::assertEquals('.style2 {font-family: \'Arial\', sans-serif; font-size: 10pt;}', $matches[0]);
         $prg = preg_match('/^[.]style3[^\\r\\n]*/m', $style, $matches);
-        self::assertNotFalse($prg);
+        self::assertSame(1, $prg);
         self::assertEquals('.style3 {font-family: \'DejaVu Sans Monospace\', monospace; font-size: 10pt;}', $matches[0]);
         $prg = preg_match('/^[.]style4[^\\r\\n]*/m', $style, $matches);
-        self::assertNotFalse($prg);
+        self::assertSame(1, $prg);
         self::assertEquals('.style4 {font-family: \'Arial\'; font-size: 10pt;}', $matches[0]);
     }
 
@@ -181,20 +181,20 @@ public function testFontNames3(): void
         self::assertEquals('style4', Helper::getTextContent($xpath, '/html/body/div/p[5]/span', 'class'));
 
         $style = Helper::getTextContent($xpath, '/html/head/style');
-        $prg = preg_match('/^[*][^\\r\\n]*/m', $style, $matches);
-        self::assertNotFalse($prg);
-        self::assertEquals('* {font-family: \'Courier New\', monospace; font-size: 12pt;}', $matches[0]);
+        $prg = preg_match('/^body[^\\r\\n]*/m', $style, $matches);
+        self::assertSame(1, $prg);
+        self::assertEquals('body {font-family: \'Courier New\', monospace; font-size: 12pt;}', $matches[0]);
         $prg = preg_match('/^[.]style1[^\\r\\n]*/m', $style, $matches);
-        self::assertNotFalse($prg);
+        self::assertSame(1, $prg);
         self::assertEquals('.style1 {font-family: \'Tahoma\'; font-size: 10pt; color: #1B2232; font-weight: bold;}', $matches[0]);
         $prg = preg_match('/^[.]style2[^\\r\\n]*/m', $style, $matches);
-        self::assertNotFalse($prg);
+        self::assertSame(1, $prg);
         self::assertEquals('.style2 {font-family: \'Arial\', sans-serif; font-size: 10pt;}', $matches[0]);
         $prg = preg_match('/^[.]style3[^\\r\\n]*/m', $style, $matches);
-        self::assertNotFalse($prg);
+        self::assertSame(1, $prg);
         self::assertEquals('.style3 {font-family: \'DejaVu Sans Monospace\', monospace; font-size: 10pt;}', $matches[0]);
         $prg = preg_match('/^[.]style4[^\\r\\n]*/m', $style, $matches);
-        self::assertNotFalse($prg);
+        self::assertSame(1, $prg);
         self::assertEquals('.style4 {font-family: \'Arial\'; font-size: 10pt;}', $matches[0]);
     }
 
@@ -221,19 +221,19 @@ public function testWhiteSpace(): void
         $xpath = new DOMXPath($dom);
 
         $style = Helper::getTextContent($xpath, '/html/head/style');
-        self::assertNotFalse(preg_match('/^[*][^\\r\\n]*/m', $style, $matches));
-        self::assertEquals('* {font-family: \'Arial\'; font-size: 12pt; white-space: pre-wrap;}', $matches[0]);
+        self::assertNotFalse(preg_match('/^body[^\\r\\n]*/m', $style, $matches));
+        self::assertEquals('body {font-family: \'Arial\'; font-size: 12pt; white-space: pre-wrap;}', $matches[0]);
         $prg = preg_match('/^[.]style1[^\\r\\n]*/m', $style, $matches);
-        self::assertNotFalse($prg);
+        self::assertSame(1, $prg);
         self::assertEquals('.style1 {font-family: \'Courier New\'; font-size: 10pt; white-space: pre-wrap;}', $matches[0]);
         $prg = preg_match('/^[.]style2[^\\r\\n]*/m', $style, $matches);
-        self::assertNotFalse($prg);
+        self::assertSame(1, $prg);
         self::assertEquals('.style2 {font-family: \'Courier New\'; font-size: 10pt;}', $matches[0]);
         $prg = preg_match('/^[.]style3[^\\r\\n]*/m', $style, $matches);
-        self::assertNotFalse($prg);
+        self::assertSame(1, $prg);
         self::assertEquals('.style3 {font-family: \'Courier New\'; font-size: 10pt; white-space: normal;}', $matches[0]);
         $prg = preg_match('/^[.]style4[^\\r\\n]*/m', $style, $matches);
-        self::assertNotFalse($prg);
+        self::assertSame(1, $prg);
         self::assertEquals('.style4 {font-family: \'Courier New\'; font-size: 10pt;}', $matches[0]);
     }
 
diff --git a/tests/PhpWordTests/Writer/HTML/Helper.php b/tests/PhpWordTests/Writer/HTML/Helper.php
@@ -64,7 +64,7 @@ public static function getNamedItem(DOMXPath $xpath, string $query, string $name
             if ($item2 === null) {
                 self::fail('Unexpected null return requesting item');
             } else {
-                $returnValue = $item2->attributes->getNamedItem($namedItem);
+                $returnVal = $item2->attributes->getNamedItem($namedItem);
             }
         }
 
@@ -94,4 +94,13 @@ public static function getAsHTML(PhpWord $phpWord, string $defaultWhiteSpace = '
 
         return $dom;
     }
+
+    public static function getHtmlString(PhpWord $phpWord, string $defaultWhiteSpace = '', string $defaultGenericFont = ''): string
+    {
+        $htmlWriter = new HTML($phpWord);
+        $htmlWriter->setDefaultWhiteSpace($defaultWhiteSpace);
+        $htmlWriter->setDefaultGenericFont($defaultGenericFont);
+
+        return $htmlWriter->getContent();
+    }
 }
diff --git a/tests/PhpWordTests/Writer/HTML/PartTest.php b/tests/PhpWordTests/Writer/HTML/PartTest.php
@@ -178,11 +178,17 @@ public function testTitleStyles(): void
         $xpath = new DOMXPath($dom);
 
         $style = Helper::getTextContent($xpath, '/html/head/style');
-        self::assertNotFalse(strpos($style, 'h1 {font-family: \'Calibri\'; font-weight: bold;}'));
+        //self::assertNotFalse(strpos($style, 'h1 {font-family: \'Calibri\'; font-weight: bold;}'));
         self::assertNotFalse(strpos($style, 'h1 {margin-top: 0.5pt; margin-bottom: 0.5pt;}'));
-        self::assertNotFalse(strpos($style, 'h2 {font-family: \'Times New Roman\'; font-style: italic;}'));
+        //self::assertNotFalse(strpos($style, 'h2 {font-family: \'Times New Roman\'; font-style: italic;}'));
         self::assertNotFalse(strpos($style, 'h2 {margin-top: 0.25pt; margin-bottom: 0.25pt;}'));
         self::assertEquals(1, Helper::getLength($xpath, '/html/body/div/h1'));
         self::assertEquals(2, Helper::getLength($xpath, '/html/body/div/h2'));
+        // code for getNamedItem had been erroneous
+        self::assertSame("font-family: 'Calibri'; font-weight: bold;", Helper::getNamedItem($xpath, '/html/body/div/h1', 'style')->textContent);
+        $html = Helper::getHtmlString($phpWord);
+        self::assertStringContainsString('<h1 style="font-family: \'Calibri\'; font-weight: bold;">Header 1 #1</h1>', $html);
+        self::assertStringContainsString('<h2 style="font-family: \'Times New Roman\'; font-style: italic;">Header 2 #1</h2>', $html);
+        self::assertStringContainsString('<h2 style="font-family: \'Times New Roman\'; font-style: italic;">Header 2 #2</h2>', $html);
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -46,8 +46,14 @@ public function write()`
`46`	`46`	`$writer = new Container($this->parentWriter, $text);`
`47`	`47`	`$text = $writer->write();`
`48`	`48`	`}`
	`49`	`+ $css = '';`
	`50`	`+ $style = \PhpOffice\PhpWord\Style::getStyle('Heading_' . $this->element->getDepth());`
	`51`	`+ if ($style !== null) {`
	`52`	`+ $styleWriter = new \PhpOffice\PhpWord\Writer\HTML\Style\Font($style);`
	`53`	`+ $css = ' style="' . $styleWriter->write() . '"';`
	`54`	`+ }`
`49`	`55`
`50`		`- $content = "<{$tag}>{$text}</{$tag}>" . PHP_EOL;`
	`56`	`+ $content = "<{$tag}{$css}>{$text}</{$tag}>" . PHP_EOL;`
`51`	`57`
`52`	`58`	`return $content;`
`53`	`59`	`}`
Original file line number	Diff line number	Diff line change
`@@ -64,7 +64,7 @@ public static function getNamedItem(DOMXPath $xpath, string $query, string $name`
`64`	`64`	`if ($item2 === null) {`
`65`	`65`	`self::fail('Unexpected null return requesting item');`
`66`	`66`	`} else {`
`67`		`- $returnValue = $item2->attributes->getNamedItem($namedItem);`
	`67`	`+ $returnVal = $item2->attributes->getNamedItem($namedItem);`
`68`	`68`	`}`
`69`	`69`	`}`
`70`	`70`
`@@ -94,4 +94,13 @@ public static function getAsHTML(PhpWord $phpWord, string $defaultWhiteSpace = '`
`94`	`94`
`95`	`95`	`return $dom;`
`96`	`96`	`}`
	`97`	`+`
	`98`	`+ public static function getHtmlString(PhpWord $phpWord, string $defaultWhiteSpace = '', string $defaultGenericFont = ''): string`
	`99`	`+ {`
	`100`	`+ $htmlWriter = new HTML($phpWord);`
	`101`	`+ $htmlWriter->setDefaultWhiteSpace($defaultWhiteSpace);`
	`102`	`+ $htmlWriter->setDefaultGenericFont($defaultGenericFont);`
	`103`	`+`
	`104`	`+ return $htmlWriter->getContent();`
	`105`	`+ }`
`97`	`106`	`}`