XLSX Charts reported as corrupt by Excel in PhpSpreadsheet.

[brandpeter]

In file PhpSpreadsheet/Writer/Xlsx/Chart.php lines 1600 and 1657 writing a "C:layout" parent element to the "c;manualLayout" cause Excel to report "We found a problem with some content in 'xxxxxx.xlsx'. Do you want us to try to recover as much as we can?". Clicking 'Yes' fixes the spreadsheet, so it is a minor annoyance but would be great if you can fix it. Commenting out those two lines removes the message.

[oleibman]

Can you upload code, or a spreadsheet, which demonstrates this problem?

[brandpeter]

LAW74.Billings.xlsx

[brandpeter]

I thought it might be more appropriate to confirm with Microsoft whether manuaLayout element should be a child of layout. All I did was save the XLSX after Excel had repaired it and compared the content of the XLSX before and after the fix by unzipping them. There were a few cosmetic changes and the one structural change as outlined. I then tested by commenting out lines 1600 and 1657 as above which resolved the error.

[oleibman]

At least 2 of our sample programs generate the manualLayout tag as yours does, and they do not wind up corrupt. So something else is going on here. One such example is samples/Chart33a/33_Chart_create_bar_custom_colors.php; another, in the same directory, is 33_Chart_create_bar_labels_lines.php. Like yours, the manualLayout tag is empty, and the layout tag would be empty except that it contains the manualLayout tag. I am, frankly, unsure what either of them is supposed to do, but I am reluctant to yank them from the code. In addition, some of the sample templates have non-empty manualLayout tags, e.g. samples/templates/32readwriteScatterChart1.xlsx, and we certainly need to leave the tests involving those unaffected if we do make a change.

It is probably (not guaranteed) safe to eliminate the manualLayout tag if it's empty. I would like to understand what's going wrong better before I embark on this course. Can you compare your code to the two samples above and see if you notice any differences between them and how you are handling Layout?

One possibility is that manualLayout is not permitted for a line chart, but I would need to see that documented somewhere. If you can figure out which chart is corrupt, and if it's possible to change it from a line chart to a bar chart in your code, and if that winds up creating a non-corrupt file, that would be an interesting result.

[oleibman]

One other test you might want to run. I believe that you commented out the c:layout tag in the code, and this resulted in a good spreadsheet for you. What happens if, instead of commenting out c:layout, you comment out the c:manualLayout section?

[oleibman]

On further inspection, I believe that the problem has nothing to do with layout nor manualLayout. It seems to be with the following:

<c:dispBlanksAs val="0"/>

When Excel fixes this, that tag becomes:

<c:dispBlanksAs val="gap"/>

Looking at the code, it appears that valid values for this attribute are gap, zero (as opposed to 0), or span. You should correct how you are setting this value. (And we should possibly omit the tag if the value isn't valid.)

[brandpeter]

Excel needs that manualLayout tag to render the chart. That element is not the issue. It looks like you either have a layout OR a manualLayout, not the one within the other. I haven't checked the XLXS spec on that.
I didn't write the val attribute of the dispBlankAs tag, that will be PHPSpreadsheet that did that. I'm not part of your dev team, just reporting what I found, and trying to be helpful. I saw other changes to the XML that Excel made, which I took to be cosmetic as I don't see any visual difference between the charts as created by PHPSpreadsheet and after Excel has had a look at it.

[oleibman]

PhpSpreadsheet does not set the value of dispBlankAs on its own. The constructor sets it by default to gap. Something else is setting it to 0. It should be fairly easy for you to determine where and how that is happening. Or, if you don't want to investigate further, just set it to gap for each chart before you write and your problem should go away.

As I said, we have many examples of manualLayout under layout, none of which result in corruption. The spec confirms that manualLayout is a child of layout. That is not the problem you are having. If you looked at the xml before and after, you will see that dispBlanksAs has changed. That is not a cosmetic change.

[brandpeter]

Ah, I thought you were talking about changing the XML attribute. Yes, of course, my code passes a 0 as the displayBlanksAs of the Chart constructor. This is old code migrated from PHPExcel written a long time ago. I think the intention was not to leave a gap in the line chart, but to plot a zero for cases where the data was blank. Restoring the commented lines in the Writer\chart and using 'gap' as the parameter in my constructor does write a valid XLSX, you are quite right.
So, Excel is converting the displayBlanksAs attribute value of '0' to 'gap' AND removing the layout element promoting the manualLayout in the XML DOM, but is happy with a value of 0 if the manualLayout is NOT a child of layout. Very strange.
I'm not sure whether I will ever get a blank in my data, so I'm happy to leave it as such. I couldn't find the acceptable values of the contructor parameters on phpspreadsheet.readthedocs.io, but will investigate when time permits.
Thank you for your attention.

[oleibman]

I will submit a PR in a day or two which doesn't allow that property to take on an invalid value. I will leave this issue open till that is complete. I just have to decide whether to throw an exception if invalid, or just ignore it. Also, whether to convert 0 to zero. Looking at the PhpExcel code, 0 was the default, but it ignored the property value and always wrote gap to the Xml (so maybe I should treat 0 as gap).

[brandpeter]

Excellent. I will change my code to use 'zero'.
Thanks again.