Merge pull request #4676 from oziriemeka/fix/4505-vml-namespace

Xlsx: register x and o namespaces for VML shapes; add test for #4505

Author: oleibman

.php-cs-fixer.dist.php

@@ -84,6 +84,8 @@
         'method_chaining_indentation' => true,
         'modernize_strpos' => true,
         'modernize_types_casting' => true,
+        'modifier_keywords' => ['elements' => ['property', 'method']], // not const
+
         'multiline_comment_opening_closing' => true,
         'multiline_whitespace_before_semicolons' => true,
         'native_constant_invocation' => false, // Micro optimization that look messy
@@ -236,7 +238,6 @@
         'types_spaces' => true,
         'unary_operator_spaces' => true,
         'use_arrow_functions' => true,
-        'visibility_required' => ['elements' => ['property', 'method']], // not const
         'void_return' => true,
         'whitespace_after_comma_in_array' => true,
         'yoda_style' => false,

src/PhpSpreadsheet/Reader/Xlsx.php

@@ -1205,7 +1205,10 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
                                     $shapes = self::xpathNoFalse($vmlCommentsFile, '//v:shape');
                                     foreach ($shapes as $shape) {
                                         /** @var SimpleXMLElement $shape */
-                                        $shape->registerXPathNamespace('v', Namespaces::URN_VML);
+                                        $vmlNamespaces = $shape->getNamespaces();
+                                        $shape->registerXPathNamespace('v', $vmlNamespaces['v'] ?? Namespaces::URN_VML);
+                                        $shape->registerXPathNamespace('x', $vmlNamespaces['x'] ?? Namespaces::URN_EXCEL);
+                                        $shape->registerXPathNamespace('o', $vmlNamespaces['o'] ?? Namespaces::URN_MSOFFICE);
 
                                         if (isset($shape['style'])) {
                                             $style = (string) $shape['style'];
@@ -1230,6 +1233,7 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
                                                 $clientData = $clientData[0];
 
                                                 if (isset($clientData['ObjectType']) && (string) $clientData['ObjectType'] == 'Note') {
+                                                    $clientData->registerXPathNamespace('x', $vmlNamespaces['x'] ?? Namespaces::URN_EXCEL);
                                                     $temp = $clientData->xpath('.//x:Row');
                                                     if (is_array($temp)) {
                                                         $row = $temp[0];

tests/PhpSpreadsheetTests/Reader/Xlsx/Issue4505Test.php

@@ -0,0 +1,41 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx;
+
+use PhpOffice\PhpSpreadsheet\Reader\Xlsx as XlsxReader;
+use PHPUnit\Framework\TestCase;
+
+class Issue4505Test extends TestCase
+{
+    private static string $file = 'tests/data/Reader/XLSX/issue.4505.namespace.xlsx';
+
+    public function testVmlProcessingWithXAndONamespaces(): void
+    {
+        $reader = new XlsxReader();
+        $spreadsheet = $reader->load(self::$file);
+        $sheet = $spreadsheet->getActiveSheet();
+
+        $comments = $sheet->getComments();
+        self::assertArrayHasKey('A1', $comments);
+        self::assertSame('right', $comments['A1']->getAlignment());
+        self::assertSame("Some User:\nHello", (string) $comments['A1']->getText());
+        $spreadsheet->disconnectWorksheets();
+    }
+
+    public function testVmlFileContainsRequiredNamespaces(): void
+    {
+        $file = 'zip://' . self::$file . '#xl/drawings/vmlDrawing1.vml';
+        $data = (string) file_get_contents($file);
+
+        self::assertStringContainsString('<ns1:shape ', $data); // usually v:shape
+        self::assertStringContainsString('<ns3:shapelayout ns1:ext="edit">', $data); // usually o:shapelayout v:ext
+        self::assertStringContainsString('<ns2:ClientData ObjectType="Note">', $data); // usually x:ClientData
+        self::assertStringContainsString('ns3:insetmode', $data); // usually o:insetmode
+        self::assertStringContainsString(
+            '<ns2:TextHAlign>Right</ns2:TextHAlign>', // usually x:TextHAlign
+            $data
+        );
+    }
+}