feat: add drawing pass-through support for unsupported elements

[rpg600]

feat: add drawing pass-through support for unsupported elements

Add opt-in pass-through mechanism to preserve unsupported drawing
elements (shapes, textboxes etc) during read/write operations.

• Add enableDrawingPassThrough property to BaseReader
• Store original drawing XML when pass-through is enabled
• Reuse stored XML in Writer instead of regenerating
• Disabled by default for backward compatibility

This allows preservation of Excel elements not yet supported by
PhpSpreadsheet without requiring full implementation.

Currently, the pass-through mechanism works as follows:

1. Reader: setEnableDrawingPassThrough(true) stores the original drawing XML
2. Writer: Automatically uses the stored XML if:
   • The drawing collection is empty (no drawings added programmatically)
   • AND the unparsed data contains the original XML

I'm open to adding an explicit Writer flag if you believe it provides better clarity and control.

This addresses existing issue : #4037

[oleibman]

I am still evaluating whether this is a good idea or not. Will it work if you add or remove a row or a column on the sheet? Add a comment when there weren't any before, and when there were? Remove a comment?

Your commentary says that you are fixing 3 open issues. Are there tests for each of those?

[rpg600]

Will it work if you add or remove a row or a column on the sheet?

Yes but the drawing's coordinates (col, row, offsets) will not be adjusted.

Add a comment when there weren't any before, and when there were? Remove a comment?

Comments are completely independent from drawings.

Your commentary says that you are fixing 3 open issues. Are there tests for each of those?

I was too quick when referencing the issues, this PR actually only addresses one of them.

I added several unit tests, including checks for unsupported elements (shapes/textboxes), behavior without pass-through, comment deletion, row/column operations, and correct flag handling between the Reader and Writer.

This feature isn’t ideal, as it’s essentially a workaround, but it will provide a temporary solution while we wait for those implementations, and it will help avoid patching the code.

[oleibman]

Thank you for adding the tests.

Comments are, alas, not completely independent from drawings. It's why I asked the question. Create a new spreadsheet with a comment. Notice that there is a drawings folder with a vml file in it in the saved spreadsheet. I can't offhand tell whether this will be a problem for your change.

[rpg600]

I have added testCommentsAndPassThroughCoexist
It shows that VML drawings and DrawingML coexist in the xl/drawings/ folder without interference. Both existing and new comments are preserved correctly when pass-through is enabled.

[oleibman]

It is not absolutely required that Coveralls doesn't report a decrease. However, we strive to eliminate that situation if possible, especially in code which is new in a change. In Writer\Xlsx\Drawing.php, you have the following new statements which include some "misses" around line 617:

        if (!isset($unparsedLoadedData['sheets'][$codeName])) {
            return null;
        }

        $sheetData = $unparsedLoadedData['sheets'][$codeName];
        if (!is_array($sheetData)) {
            return null;
        }

        // Only use pass-through XML if the Reader flag was explicitly enabled
        if (($sheetData['drawingPassThroughEnabled'] ?? false) !== true) {
            return null;
        }

        if (!isset($sheetData['Drawings']) || !is_array($sheetData['Drawings'])) {
            return null;
        }

I tthink if you changed that block as follows, your code would still work as planned and there would no longer be any coverage misses:

        $sheetData = $unparsedLoadedData['sheets'][$codeName] ?? null;
        // Only use pass-through XML if the Reader flag was explicitly enabled
        if (!is_array($sheetData) || ($sheetData['drawingPassThroughEnabled'] ?? false) !== true || !is_array($sheetData['Drawings'] ?? null)) {
            return null;
        }

Please give that a shot.

[oleibman]

Thank you - success on both fronts - no problems with unit tests, and no missed statements.

[rpg600]

Hi @oleibman,

Thanks for your feedback.
I was wondering if you’re still interested in merging this feature. If so, do you have a timeline in mind?

[oleibman]

I am probably interested in merging this. I do not have a timeline in mind, but I imagine it could happen before year-end. It is complicated, and I need to understand it better than I do at the moment, because once I merge it, I have to maintain it.

[oleibman]

In issue #4037, there is a spreadsheet attached:
https://github.com/PHPOffice/PhpSpreadsheet/files/15400250/merge.excel.xlsx
It has a photograph, a textbox, a drawing of glasses, and a blue shape.

When I load and save that with the existing PhpSpreadsheet code, and open the result in Excel, I see the photograph and the glasses, but the textbox and shape are missing. The missing parts are, of course, why you are writing the PR.

When I load and save that with your new code (making sure to setEnableDrawingPassThrough(true), I now see the textbox and the shape, as well as the glasses. But the photograph - Excel says the picture cannot be displayed! Do you see a different result when you try it?

[rpg600]

@oleibman You're right — I observed the same behavior.

The issue is with SVG images (the glasses in merge.excel.xlsx): Excel stores both the SVG and a PNG fallback with separate rIds (e.g., rId1=PNG fallback, rId2=SVG). Since PhpSpreadsheet doesn't load SVG images into the drawing collection, only the PNG fallback is written to the relations file. This causes the rIds to shift and no longer match the original pass-through drawing XML.

For example:

• Original: rId1=image1.png, rId2=image2.svg, rId3=image3.jpeg
• After save: rId1=image1.png, rId2=image3.jpeg (shifted)
• But the XML still references rId3 for image3.jpeg → broken

One solution would be to also pass through the original drawing relationships and media files. WDYT ?

[oleibman]

Another solution might be to pass through the SVG file as a drawing, in the same manner as you are passing through other files. This is just a thought off the top of my head. I have no idea how practical or difficult that would be, especially compared to your proposal.

[rpg600]

Thank you for your suggestion. I analyzed this approach, but it turns out SVGs in Excel are not standalone drawings, they're embedded as extensions within <a:blip> elements using <asvg:svgBlip>.

It looks like this:

<a:blip r:embed="rId1">
  <a:extLst>
    <a:ext uri="{...}">
      <asvg:svgBlip r:embed="rId2"/>  <!-- SVG reference inside PNG blip -->
    </a:ext>
  </a:extLst>
</a:blip>

The SVG and its PNG fallback are bound together in one drawing element. Supporting separate SVG drawings would require a major refactoring IMO.

The simpler solution is to extend pass-through to also preserve:

1. The original drawing relationships (to keep rId alignment)
2. The original media files (to copy SVGs from source)

This approach:

• Keeps changes minimal
• Doesn't break existing drawing logic
• Works for any unsupported media format

I've implemented this fix with tests using merge.excel.xlsx

[oleibman]

You are getting very close. Still, there is a problem. When I load and save merge.excel.xlsx using your PR and setEnableDrawingPassThrough, Excel complains when I try to open the result. If I let it do whatever it thinks is needed to clean up, the result is as desired - the photo, the glasses, the textbox, and the shape are all there. If you can figure out and avoid whatever it is Excel is objecting to, I think I will be able to merge this.