* Upgrade Dev TCPDF to 6.5
Implementation of https://github.com/tecnickcom/TCPDF/pull/467, which is available in just-released Tcpdf 6.5, will improve look of Tcpdf rendering for PhpSpreadsheet. Fix#1164.
One test had been suppressed for Tcpdf, ostensibly because it was not compatible with Php8. As it turns out, the PhpSpreadsheet code which invokes Tcpdf was (harmlessly) incorrect, so the Php8 issue was actually with PhpSpreadsheet, not Tcpdf. That code is corrected, and the test is no longer suppressed.
* Update Change Log
Pick up some earlier changes as well as this one, and deprecations which had been omitted from the 1.24 change log.
See discussion in #2999. Mpdf is not acknowledging the styling that we're using to hide table rows. I have opened an issue with them, but enclosing the cells in the hidden row inside a div with appropriate css does seem to be a workaround, and that can be incorporated into PhpSpreadsheet. It's kludgey, and it isn't even valid HTML, but ...
Mpdf also doesn't like the addition of the ```file:///``` prefix when using local images from Windows (sample 21). Results are better when that prefix is not added.
Dompdf seemed to have problems with sample 21 images on both Windows and Unix, with or without the file prefix. It does, however, support data urls for both, so is changed to embed images. It's still not perfect - the image seems truncated to the row height - but the results are better. I will continue to research, but may proceed as-is if I don't find anything better to do.
Html Writer was producing a file with mixed line endings on Windows. This didn't cause any harm, but it seems a bit sloppy. It is changed to always use PHP_EOL as a line ending.
* Replace Dev jpgraph/jpgraph with mitoteam/jpgraph
PR #2979 added support for mitoteam/jpgraph as an alternative to jpgraph/jpgraph. The package jpgraph/jpgraph is abandoned in composer, and the version loaded with composer has been unusable for some time. This PR removes the dev requirement for jpgraph/jpgraph, and adds a dev requirement for mitoteam/jpgraph in its place.
With a usable graph library, a number of tests and samples that had been disabled are now re-enabled. A lot of new functionality has been added to Charts recently. Some of that new code has exposed bugs in JpgraphRendererBase. I have fixed those where I could. A handful of exceptions remain; I will investigate, and hopefully fix, those over time, but I don't feel it is necessary to fix them all before installing this PR - we are already way ahead of the game with the graphs that are working.
Three members had been ignoring code coverage in whole or in part because of the unavailability of a usable graph libray. Code coverage is restored in them. I am relieved to report that, although they aren't completely covered, adding them did not reduce code coverage by much - it is still over 90.4%.
I took a look at JpgraphRendererBase and Phpstan. Phpstan reports 128 problems. When I added some docblocks to correct some of those, the number increased to 284. Sigh. I will investigate over time, but, for now, we will still suppress Phpstan for JpgraphRendererBase.
I do not find a License file for mitoteam. However, there also wasn't one for jpgraph in the first place. Based on that and the discussion in #2996 (mitoteam will be used in exactly the same manner as mpdf), I don't think this is a problem. IANAL.
* PHP 8.2 Problems
Tons of "cannot create dynamic property" deprecations in jpgraph. Disable the test with most of those for now; leave the two with only a handful of messages enabled.
* Correct Failures in 2 Stock Charts
Down to 6 templates on which Render fails.
Addresses a problem identified in issue #2991. Chart name is set in constructor, but there is no method to subsequently change it. This PR adds a method to do so.
* Test if UConverter Exists Without Autoload
Fix#2982. That issue is actually closed, but it did expose a problem. Our test environments all enable php-intl, but that extension isn't a formal requirement for PhpSpreadsheet. Perhaps it ought to be. Nevertheless ...
Using UConverter for string translation solved some problems for us. However, it is only available when php-intl is enabled. The code tests if it exists before using it, so no big deal ... except it seems likely that the people reporting the issue not only did not have php-intl, but they do have their own autoloader which issues an exception when the class isn't found. The test for existence of UConverter defaulted to attempting to autoload it if not found. So, on a system without php-intl but with a custom autoloader, there is a problem. Code is changed to suppress autoload when testing UConverter existence.
Pending this fix, the workaround for this issue is to enable php-intl.
* Minor Improvement
Make mb_convert_encoding use same substitution character as UConverter, ensuring consistent results whatever the user's environment.
* And Now That I Figured That Out
Since mb_convert_encoding can now return the same output as UConverter, we don't need UConverter (or iconv) after all in sanitizeUTF8.
No source code changes, just additional tests. FormulaParser appears unused, replaced by newer code in Calculation. However, it's a public interface, so probably shouldn't be deleted without first deprecating it. I have no strong feelings about whether that should happen. However, as long as it's part of the package, we may as well have some formal unit tests for it.
This is a start in addressing issue #2965 (and earlier issue #749). Chart Titles are usually entered as strings or Rich Text strings, and PhpSpreadsheet supports that. They can also be entered as formulas (typically a pointer to a cell with the title text), and, not only did PhpSpreadsheet not support that, it threw an exception when reading a spreadsheet that did so.
This change does:
- eliminate the exception
- set a static chart title when it can determine it from the Xml
This change does not:
- fully support dynamic titles (e.g. if you change the contents of the source cell, or delete or insert cells or rows or columns)
- permit the user to set the title to a formula
- allow the use of formulas when writing a chart title to a spreadsheet
- provide styling for titles when it has read them as a formula
* Ensure multiplication is performed on a non-array value
* Simplify formula
Numbers should be numbers
* Provide test coverage for SUM combined with INDEX/MATCH
* PHPStan
Shared/Font is hardly covered in unit tests (as opposed to Style/Font which is completely covered). And it presented some good opportunities for code optimization. I wrote and tested the new unit tests first, then optimized the code and confirmed that everything still works.
There is still a bit of a gap with "exact" measurements. I had tests ready, but had to withdraw them when I discovered they weren't quite portable (see https://github.com/php/php-src/issues/9073).
* Fix Some Pdf Problems
Fix#1747. No support for text rotation in Pdf. That issue actually has a decent workaround, but PhpSpreadsheet should handle it on its own. Mpdf requires the proprietary text-rotate css attribute; Html and Dompdf will use the CSS3 attribute transform:rotate.
Fix#1713. Some paper-size values in PhpSpreadsheet are strings, some are 2-element float arrays. Dompdf accepts strings or 4-element float arrays, where the first 2 elements are always 0. Convert the PhpSpreadsheet array accordingly before passing it to Dompdf.
Some tests had been disabled when Dompdf and Tcpdf were slow to achieve PHP8 compliance. They achieved it some time ago. Re-enable the tests.
* Remove Tcpdf From One Test
No problem with the other tests I added it in for.
* Fix Spreadsheet Copy, Disable Clone, Improve Coverage
This PR was supposed to be merely to increase coverage in Spreadsheet. However, in doing so, I discovered that neither clone nor copy worked correctly. Neither had been covered in the test suite. Copy not only did not work, it broke the source spreadsheet as well. I tried to debug and got nowhere; I even tried using myclabs/deep-copy which is already in use in the test suite, but it failed as well. However, write and reload ought to work just fine for copy. It can't be used for clone; however, since copy does what clone ought to do, there's no reason why clone needs to be used, so __clone is changed to throw an exception if attempted.
One other source change was needed, an obvious bug where an if condition uses 'or' when it should use 'and'. Also, one docblock declaration needed a change. Aside from that, the rest of this PR is test cases, and overall coverage passes 89% for the first time.
* Clone is Okay After All
But copy wasn't, changing it to just return clone. Perhaps save and reload will be needed instead at some point, but not yet.
* An Error I Cannot Reproduce
PHP8.1 unit test says error because GdImage can't be serialized. I can't reproduce this error on any of my test systems. I have no idea why GdImage is even involved. Using try/catch to see if it helps.
* Weird Failures in Github
I thought restoring clone was a good idea. That left me in a state where, after one change, copy/clone no longer worked on Github (unable to reproduce on any of my test systems). After a second change, copy worked but clone didn't, again unable to reproduce. So, reverting to original version - copy does save and reload, clone throws exception.
Fix#2257. Fix#2929. Fix#2935 (probably in a way that will not satisfy the requester). 2257 and 2929 requested changes that ultimately affect the same section of code, so it's appropriate to deal with them together. 2257 requests the ability to make the chart background transparent (so that the Excel gridlines are visible beneath the chart), and the ability to hide an Axis. 2929 requests the ability to set a gradient background on the chart.
* Big Memory Leak in One Test
Many tests leak a little by not issuing disconnectWorksheets at the end. CellMatcherTest leaks a lot by opening a spreadsheet many times. Phpunit reported a high watermark of 390MB; fixing CellMatcherTest brings that figure down to 242MB. I have also changed it to use a formal assertion in many cases where markTestSkipped was (IMO inappropriately) used.
* Another Leak
Issue2301Test lacks a disconnect. Adding one reduces HWM from 242MB to 224.
Fix#2705. Add to Axis class, Reader Xlsx Chart, and Writer Xlsx Chart. Add feature to an existing 32* sample, to an existing 33* sample, and a formal unit test.
Fix#2934. Null is passed to StringHelper::strtolower which expects string. Same problem appears to be applicable to HLOOKUP.
I noted the following problem in the code, but will document it here as well. Excel's results are not consistent when a non-numeric string is passed as the third parameter. For example, if cell Z1 contains `xyz`, Excel will return a REF error for function `VLOOKUP(whatever,whatever,Z1)`, but it returns a VALUE error for function `VLOOKUP(whatever,whatever,"xyz")`. I don't think PhpSpreadsheet can match both behaviors. For now, it will return VALUE for both, with similar results for other errors.
While studying the returned errors, I realized there is something that needs to be deprecated. `ExcelError::$errorCodes` is a public static array. This means that a user can change its value, which should not be allowed. It is replaced by a constant. Since the original is public, I think it needs to stay, but with a deprecation notice; users can reference and change it, but it will be unused in the rest of the code. I suppose this might be considered a break in functionality (that should not have been allowed in the first place).
Fix#2931. If surface charts are written with c:grouping tag, Excel will treat them as corrupt. Also, some 3D-ish Xml tags are required when 2D surface charts are written, else they will be treated as 3D. Also eliminate a duplicate line identified by #2932.
* Html Reader Not Handling non-ASCII Data Correctly
Fix#2942. Code was changed by #2894 because PHP8.2 will deprecate how it was being done. See linked issue for more details. Dom loadhtml assumes ISO-8859-1 in the absence of a charset attribute or equivalent, and there is no way to override that assumption. Sigh. The suggested replacements are unsuitable in one way or another. I think this will work with minimal disruption (replace ampersand, less than, and greater than with entities representing illegal characters, then use htmlentities, then restore ampersand, less than, and greater than).
* Better Implementation
Use regexp to escape non-ASCII. Less kludgey, less reliant on the vagaries of the PHP maintainers.
* Additional Tests
Test non-ASCII outside of cell contents: sheet title, image alt attribute.
* Apply Same Change in Second Location
Forgot to change loadFromString.
* Additional Test
Confirm escaped ampersand is handled correctly.
This was supposed to be mopping up some longstanding chart issues. But one of the sample files exposed a memory leak in Xlsx Writer, unrelated to charts. Since that is my best sample file for this problem, I would like to fix both problems at the same time.
Xlsx Writer for Worksheets calls getRowDimension for all rows on the sheet. As it happens, the sample file had data in the last rows after a huge gap of rows without any data. It correctly did not write anything for the unused rows. However, the call to getRowDimension actually creates a new RowDimension object if it doesn't already exist, and so it wound up creating over a million totally unneeded objects. This caused it to run out of memory when I tried to make a copy of the 8K input file. The logic is changed to call getRowDimension if and only if (there is data in the row or the RowDimension object already exists). It still has to loop through a million rows, but it no longer allocates the unneeded storage.
As for the Chart problems - fix#1797. This is where the file that caused the memory leak originated. Many of its problems were already resolved by the earlier large set of changes to Charts. However, there were a few new properties that needed to be added to Layout to make things complete - numberFormat code and source-linked, and dLblPos (position for labels); and autoTitleDeleted needs to be added to Charts.
Also fix#2077, by allowing the format to be specified in the Layout rather than the DataSeriesValues constructor.
Mark Baker
oleibman
Mikhail Oleynik
Jonathan Goode
dependabot[bot]