Skip to content

Commit 29c0f21

Browse files
oleibmanoleibman
authored
Merge pull request #4122 from oleibman/issue4105
Better Handling of legacyDrawing Xml
2 parents bc0460d + 362cdaf commit 29c0f21

File tree

4 files changed

+194
-2
lines changed

4 files changed

+194
-2
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ and this project adheres to [Semantic Versioning](https://semver.org).
2929

3030
### Fixed
3131

32-
- Nothing yet.
32+
- Better Handling of legacyDrawing Xml. [Issue #4105](https://github.com/PHPOffice/PhpSpreadsheet/issues/4105) [PR #4122](https://github.com/PHPOffice/PhpSpreadsheet/pull/4122)
3333

3434
## 2024-08-07 - 2.2.2
3535

‎src/PhpSpreadsheet/Reader/Xlsx.php

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1640,7 +1640,9 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
16401640
foreach ($xmlSheet->legacyDrawing as $drawing) {
16411641
$drawingRelId = (string) self::getArrayItem(self::getAttributes($drawing, $xmlNamespaceBase), 'id');
16421642
if (isset($vmlDrawingContents[$drawingRelId])) {
1643-
$unparsedLoadedData['sheets'][$docSheet->getCodeName()]['legacyDrawing'] = $vmlDrawingContents[$drawingRelId];
1643+
if (self::onlyNoteVml($vmlDrawingContents[$drawingRelId]) === false) {
1644+
$unparsedLoadedData['sheets'][$docSheet->getCodeName()]['legacyDrawing'] = $vmlDrawingContents[$drawingRelId];
1645+
}
16441646
}
16451647
}
16461648
}
@@ -2373,4 +2375,36 @@ private function processIgnoredErrors(SimpleXMLElement $xml, Worksheet $sheet):
23732375
}
23742376
}
23752377
}
2378+
2379+
private static function onlyNoteVml(string $data): bool
2380+
{
2381+
$data = str_replace('<br>', '<br/>', $data);
2382+
2383+
try {
2384+
$sxml = @simplexml_load_string($data);
2385+
} catch (Throwable) {
2386+
$sxml = false;
2387+
}
2388+
2389+
if ($sxml === false) {
2390+
return false;
2391+
}
2392+
$shapes = $sxml->children(Namespaces::URN_VML);
2393+
foreach ($shapes->shape as $shape) {
2394+
$clientData = $shape->children(Namespaces::URN_EXCEL);
2395+
if (!isset($clientData->ClientData)) {
2396+
return false;
2397+
}
2398+
$attrs = $clientData->ClientData->attributes();
2399+
if (!isset($attrs['ObjectType'])) {
2400+
return false;
2401+
}
2402+
$objectType = (string) $attrs['ObjectType'];
2403+
if ($objectType !== 'Note') {
2404+
return false;
2405+
}
2406+
}
2407+
2408+
return true;
2409+
}
23762410
}

‎src/PhpSpreadsheet/Spreadsheet.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1581,4 +1581,14 @@ public function getExcelCalendar(): int
15811581
{
15821582
return $this->excelCalendar;
15831583
}
1584+
1585+
public function deleteLegacyDrawing(Worksheet $worksheet): void
1586+
{
1587+
unset($this->unparsedLoadedData['sheets'][$worksheet->getCodeName()]['legacyDrawing']);
1588+
}
1589+
1590+
public function getLegacyDrawing(Worksheet $worksheet): ?string
1591+
{
1592+
return $this->unparsedLoadedData['sheets'][$worksheet->getCodeName()]['legacyDrawing'] ?? null;
1593+
}
15841594
}

‎tests/PhpSpreadsheetTests/Reader/Xlsx/VmlTest.php

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx;
6+
7+
use PhpOffice\PhpSpreadsheet\Reader\Xlsx as XlsxReader;
8+
use PhpOffice\PhpSpreadsheet\Shared\File;
9+
use PhpOffice\PhpSpreadsheet\Spreadsheet;
10+
use PhpOffice\PhpSpreadsheet\Writer\Xlsx as XlsxWriter;
11+
use PHPUnit\Framework\TestCase;
12+
13+
class VmlTest extends TestCase
14+
{
15+
private string $outfile1 = '';
16+
17+
private string $outfile2 = '';
18+
19+
protected function tearDown(): void
20+
{
21+
if ($this->outfile1 !== '') {
22+
unlink($this->outfile1);
23+
$this->outfile1 = '';
24+
}
25+
if ($this->outfile2 !== '') {
26+
unlink($this->outfile2);
27+
$this->outfile2 = '';
28+
}
29+
}
30+
31+
public function testAddComments(): void
32+
{
33+
$spreadsheet = new Spreadsheet();
34+
$sheet = $spreadsheet->getActiveSheet();
35+
$sheet->getComment('A1')->getText()->createText('top left cell');
36+
$writer = new XlsxWriter($spreadsheet);
37+
$this->outfile1 = File::temporaryFileName();
38+
$writer->save($this->outfile1);
39+
$spreadsheet->disconnectWorksheets();
40+
41+
$reader = new XlsxReader();
42+
$file = 'zip://' . $this->outfile1 . '#xl/worksheets/sheet1.xml';
43+
$sheetContents = file_get_contents($file) ?: '';
44+
self::assertStringContainsString('<legacyDrawing ', $sheetContents);
45+
$file = 'zip://' . $this->outfile1 . '#xl/drawings/vmlDrawing1.vml';
46+
$vmlContents = file_get_contents($file) ?: '';
47+
$count = substr_count($vmlContents, '<v:shape ');
48+
self::assertSame(1, $count);
49+
$count = substr_count($vmlContents, '<x:ClientData ObjectType="Note">');
50+
self::assertSame(1, $count);
51+
52+
$spreadsheet2 = $reader->load($this->outfile1);
53+
$sheet2 = $spreadsheet2->getActiveSheet();
54+
self::assertSame('top left cell', $sheet2->getComment('A1')->getText()->getPlainText());
55+
self::assertNull($spreadsheet2->getLegacyDrawing($sheet2));
56+
$sheet2->getComment('H1')->getText()->createText('Show me');
57+
$sheet2->getComment('H2')->getText()->createText('Hide me');
58+
$sheet2->getComment('H1')->setVisible(true);
59+
$writer = new XlsxWriter($spreadsheet2);
60+
$this->outfile2 = File::temporaryFileName();
61+
$writer->save($this->outfile2);
62+
$spreadsheet2->disconnectWorksheets();
63+
64+
$file = 'zip://' . $this->outfile2 . '#xl/worksheets/sheet1.xml';
65+
$sheetContents = file_get_contents($file) ?: '';
66+
self::assertStringContainsString('<legacyDrawing ', $sheetContents);
67+
$file = 'zip://' . $this->outfile2 . '#xl/drawings/vmlDrawing1.vml';
68+
$vmlContents = file_get_contents($file) ?: '';
69+
$count = substr_count($vmlContents, '<v:shape ');
70+
self::assertSame(3, $count);
71+
$count = substr_count($vmlContents, '<x:ClientData ObjectType="Note">');
72+
self::assertSame(3, $count);
73+
74+
$reader = new XlsxReader();
75+
$spreadsheet3 = $reader->load($this->outfile2);
76+
$sheet3 = $spreadsheet3->getActiveSheet();
77+
self::assertSame('top left cell', $sheet3->getComment('A1')->getText()->getPlainText());
78+
self::assertSame('Show me', $sheet3->getComment('H1')->getText()->getPlainText());
79+
self::assertSame('Hide me', $sheet3->getComment('H2')->getText()->getPlainText());
80+
self::assertNull($spreadsheet3->getLegacyDrawing($sheet3));
81+
self::assertTrue($sheet3->getComment('H1')->getVisible());
82+
self::assertFalse($sheet3->getComment('H2')->getVisible());
83+
$spreadsheet3->disconnectWorksheets();
84+
}
85+
86+
public function testDeleteNullLegacy(): void
87+
{
88+
$spreadsheet = new Spreadsheet();
89+
$sheet = $spreadsheet->getActiveSheet();
90+
$sheet->getComment('A1')->getText()->createText('top left cell');
91+
self::assertNull($spreadsheet->getLegacyDrawing($sheet));
92+
$spreadsheet->deleteLegacyDrawing($sheet);
93+
$writer = new XlsxWriter($spreadsheet);
94+
$this->outfile1 = File::temporaryFileName();
95+
$writer->save($this->outfile1);
96+
$spreadsheet->disconnectWorksheets();
97+
98+
$reader = new XlsxReader();
99+
$file = 'zip://' . $this->outfile1 . '#xl/worksheets/sheet1.xml';
100+
$sheetContents = file_get_contents($file) ?: '';
101+
self::assertStringContainsString('<legacyDrawing ', $sheetContents);
102+
$file = 'zip://' . $this->outfile1 . '#xl/drawings/vmlDrawing1.vml';
103+
$vmlContents = file_get_contents($file) ?: '';
104+
$count = substr_count($vmlContents, '<v:shape ');
105+
self::assertSame(1, $count);
106+
$count = substr_count($vmlContents, '<x:ClientData ObjectType="Note">');
107+
self::assertSame(1, $count);
108+
109+
$spreadsheet2 = $reader->load($this->outfile1);
110+
$sheet2 = $spreadsheet2->getActiveSheet();
111+
self::assertSame('top left cell', $sheet2->getComment('A1')->getText()->getPlainText());
112+
$spreadsheet2->disconnectWorksheets();
113+
}
114+
115+
public function testAddCommentDeleteFormControls(): void
116+
{
117+
$infile = 'samples/Reader2/sampleData/formscomments.xlsx';
118+
$reader = new XlsxReader();
119+
$reader->setLoadSheetsOnly('FormsComments');
120+
$spreadsheet = $reader->load($infile);
121+
self::assertTrue(true);
122+
$sheet = $spreadsheet->getActiveSheet();
123+
self::assertSame('row1', $sheet->getCell('H1')->getValue());
124+
self::assertStringContainsString('Hello', $sheet->getComment('F1')->getText()->getPlainText());
125+
$vmlContents = $spreadsheet->getLegacyDrawing($sheet) ?? '';
126+
$count = substr_count($vmlContents, '<v:shape ');
127+
self::assertSame(4, $count);
128+
$count = substr_count($vmlContents, '<x:ClientData ');
129+
self::assertSame(4, $count);
130+
$count = substr_count($vmlContents, '<x:ClientData ObjectType="Note"');
131+
self::assertSame(1, $count);
132+
$spreadsheet->deleteLegacyDrawing($sheet);
133+
$sheet->getComment('F2')->getText()->createText('Goodbye');
134+
$writer = new XlsxWriter($spreadsheet);
135+
$this->outfile1 = File::temporaryFileName();
136+
$writer->save($this->outfile1);
137+
$spreadsheet->disconnectWorksheets();
138+
139+
$reader2 = new XlsxReader();
140+
$spreadsheet2 = $reader2->load($this->outfile1);
141+
$sheet2 = $spreadsheet2->getActiveSheet();
142+
self::assertNull($spreadsheet2->getLegacyDrawing($sheet2));
143+
self::assertSame('row1', $sheet2->getCell('H1')->getValue());
144+
self::assertStringContainsString('Hello', $sheet2->getComment('F1')->getText()->getPlainText());
145+
self::assertStringContainsString('Goodbye', $sheet2->getComment('F2')->getText()->getPlainText());
146+
$spreadsheet2->disconnectWorksheets();
147+
}
148+
}

0 commit comments

Comments
 (0)