Fix#1810, just before it turns 6 years old. Reporter thinks it can save time. Easy to implement, especially with the advent of named parameters which were not available when issue was opened.
Issue3982Test mysteriously ran into memory problems when we migrated from Phpunit 9 to 10. It wasn't all that critical a test, so it has been disabled ever since. I finally had some time to research, and the problem is unquestionably with Phpunit's `assertCount` test - it doesn't like something about our array. However, we can easily redo that test by using Php's native `count` function, and testing that result with `assertSame`. I have not yet succeeded at simplifying the test to a state where I am willing to report the bug, but I'll keep trying. In the meantime, the test is recoded, and can now be run successfully. No source code changes.
This will improve round-tripping from Xlsx to Html and back again. The calculated result will still be what is displayed when viewing the result in a browser, but, using the new option, the original formula can be reconstructed when PhpSpreadsheet reads the Html file. The ability to recognize this situation is automatically enabled in Html Reader, however, to avoid BC issues, it needs to be explicitly enabled for Html Writer. A new boolean property `dataFormula` and setter are added to Writer; it will default to current behavior `false` for now, but my plan is to change the default in the next breaking release. I cannot think of a decent use case for a getter, so have not added one.
Fix#4280, an issue with many problems; this is the last one (PR #4281 and PR #4677 dealt with other aspects of the problem). New releases of Excel allow representing TRUE/FALSE as a checked/unchecked checkbox. This requires a change to Styles, as expected. But Excel has implemented it in such a manner that changes are also required to Content_Types and workbook.xml.rels; and a new xml file propertyFeatureBag is also required. Seems like a lot of work for such a small change. A new property is added to Style, but only Xlsx Reader and Writer support it; no other format does.
While reviewing the DefaultValueBinder::dataTypeForValue() method, I noticed that a new Calculation instance is being created for every single cell validation.
Since the Calculation class is quite heavy (initializing components like CyclicReferenceStack and BranchPruner), instantiating it repeatedly causes unnecessary memory and CPU overhead, especially during bulk imports of large spreadsheets.
Changes:
Switched from new Calculation() to Calculation::getInstance() to leverage the existing singleton.
Added state preservation logic for branchPruning to ensure the shared instance remains consistent for other parts of the application.
Wrapped the logic in a finally block to guarantee that the pruning state is restored regardless of success or failure.
No changes were made to the public API, so this is fully backward compatible. I've also added a unit test to verify that formula detection and state isolation work as expected.
Tests:
Added tests/PhpSpreadsheetTests/Cell/DefaultValueBinderTest.php.
The table attributes `border`, `cellspacing`, and `cellpadding` are used in Html Writer. They are deprecated in Html5 in favor of css attributes. Further, their use is redundant or outright wrong. Their use is eliminated. A sample is added to show how equivalents might be used.
It currently always uses PHP_EOL, which, of course, works, but you get slightly different results in Windows and Unix. User can now set line ending to `\n` or `\r\n` to ensure consistent results regardless of environment. Default remains PHP_EOL. We already do this for CSV.
Fix#4773. Mpdf is applying cell style only to the first line of a multi-line string. Explanation - Html Writer replaces newlines with `<br />` by calling `nl2br`, at least that was the intention. However, `nl2br` doesn't replace - it prepends. This doesn't do any harm in Html, Dompdf, or Tcpdf. However, Mpdf is known to be subject to occasional regexp backtracking errors when parsing large blocks of html. To avoid this, we chunk the html by splitting it at newlines, but this causes the styling to be lost for rich text elements following a newline.
The solution to this problem seems easy enough - replace `nl2br` with a routine that replaces rather than prepends. Unfortunately, Html Reader needs the newlines intact in order to parse things correctly. So a solution where the `nl2br` calls are used for non-Mpdf and using a substitute routine for Mpdf ought to work. However, doing my testing uncovered another bug - rich text with newlines can result in multiple `<br />` tags (see new Html test). The solution to this is a bit kludgey, but it does seem to fix both the Mpdf problem and the newly discovered Html problem. It will also tend to avoid the very minor problem of generating different line endings on Windows systems.
Fix#606 (marked stale in 2018, but now reopened). User reports that, under some circumstances, `oldCalculatedValue` will be used as the result when an unsupported function is part of a formula (this seems like a sensible treatment), but, in other cases, the unsupported function is ignored, or has an unpredictable effect on the result.
To explain a little further, an unsupported function will return a "magic" value which will eventually be replaced by `oldCalculatedValue` (or null if there is none). However, the magic value is not being propagated. In the issue, there are 3 places where this propagation is currently omitted - when performing a binary comparison, when evaluating the condition in the `IF` function, and when evaluating the `SUM` function. This PR causes propagation to happen in all of those places, and a few others (MIN, MINA, MAX, MAXA, ROWS, COLUMNS). I would not be surprised to have missed one or more of the other situations.
Fix#3889. Fix#2464. After evaluating HYPERLINK function specifying cell address(es) rather than literal(s), the calculated hyperlink is attached to the wrong cell. I probably should have figured this out sooner - this is just another instance of the common problem of trying to assign a cell to a variable and then using the variable after a delay during which the "current" cell may have changed. The problem is resolved by storing the cell's worksheet and coordinate on entry to HYPERLINK, then restoring those before manipulating the hyperlink before returning to the caller.
An additional problem with hyperlinks is that assigning a new value to a cell has not cleared any hyperlink associated with the cell. This will now happen, if needed, whenever `setValue` or `setValueExplicit` is called.
A third problem is unaddressed. A (fairly unrealistic) formula like:
```php
$sheet->getCell('A4')->setValue('=LEN(HYPERLINK("http://www.example.com", "Example"))');
```
winds up attaching a hyperlink to the cell when its value is calculated, and it probably shouldn't. However, @TobiasBg reports in 2464 that Excel for Mac does the same thing, and I've just confirmed that Excel 365 on Windows does likewise. Furthermore, the hyperlink which Excel creates is not usable ("Cannot open the specified file"), whereas the link that PhpSpreadsheet creates is usable. So we're not doing any worse than Excel, and arguably doing better. I'm satisfied.
Thank you to @raziel057, who pointed out this problem while doing some research on a PR on which he is working. The code in AxisShadowTest generates a spreadsheet which, if written to disk, will be treated by Excel as corrupt. Since the test doesn't actually generate a spreadsheet, it isn't entirely invalid, but it should do a better job of presenting usable code.
Chart axes have a shadow property, which has many sub-properties. One of these is `size`, which also has sub-properties. Two of those are `kx` and `ky`, representing the skew angle of the shadow. These must be < 90 degrees and > -90 degrees (strangely *not* <= and >=) (see https://ooxml.info/docs/20/20.1/20.1.10/20.1.10.23/). If the supplied value is outside that range, Excel treats the chart as corrupt. That is the main problem with AxisShadowTest - kx was supplied as `-94`. This problem is easily solved in the test by setting it to a valid value instead. And, of course, Chart Writer is changed to not output the value if it is out of range.
Size also has 2 other properties `sx` and `sy`, where a value of 1 indicates 100% of normal size. The test sets `sy` to `254`, a not-particularly sane value. I am fairly certain that `2.54` was intended. I do not believe Excel treats `254` as corrupt, but the test is also changed to supply a sensible value. I imagine that there is a maximum value that Excel will allow, but I don't see it documented anywhere. The UI restricts it to 2 (200%), but larger values are accepted in the xml, so, at least for now, I allow anything.
I have to admit that the Excel UI puzzles me with respect to these properties. There is a single value that can be set for `size` (i.e. no separate property for `sx` and `sy`), and I don't see any way to set the skew angle. There is a settable `angle` property, but that refers to a different shadow property `direction`. At any rate, Excel seems to respond to changes in the xml for all of `sx`, `sy`, `kx`, and `ky`, so we will continue to support them.
oleibman
xannyprox
Thomas Lallement
Thomas Lallement