* Html Writer - Do Not Generate background-color When Fill is None
For PR #3002, I noted that there was a problem with Dompdf truncating images. I raised an issue with them (https://github.com/dompdf/dompdf/issues/2980), and they agree that there is a bug; however, they also suggested a workaround, namely omitting background-color from any cells which the image overlays. That did not at first appear to be a solution which could be generalized for PhpSpreasheet. However, investigating further, I saw that Html Writer is generating background-color for all cells, even though most of them use the default Fill type None (which suggests that background-color should not be specified after all). So this PR changes HTML Writer to generate background-color only when the user has actually set Fill type to something other than None. This is not a complete workaround for the Dompdf problem - we will still see truncation if the image overlays a cell which does specify a Fill type - however, it is almost certainly good enough for most use cases.
In addition to that change, I made the generated Html a little smaller and the code a little more efficient by combining the TD and TH styles for each cell into a single declaration and calling createCssStyle only once.
* Revamp One Test
Look for both td.style and th.style instead of just td.style in test.
* 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.
Drawings in an Xlsx file are stored in such a way that Php can read their contents using the `zip:` protocol. This does not, however, work when the file is read by PhpSpreadsheet and then saved as Html or Pdf, since the browser will not recognize that protocol even if the file is available. Such drawings need to be saved in the html as embedded images in order for the copy to display them properly. This is true even when the writer is set to not embed images (default).
An additional problem arises when an Html file with an embedded image is read, because `Worksheet\Drawing::setPath` attempts to validate the path, which it cannot do for the `data:image` Url which embedded images use.
And yet another problem. Writer/Html writes out a MemoryDrawing as a png using the imagepng function; but then declares it as jpeg in the Html. This is now corrected.
And a fourth problem. Writer/Html ignores the last row if it contains nothing but a Memory Drawing, which can be true when copying an Xls file.
These changes are testable (it's how I discovered the second part of this parlay). I think it is also useful to add a sample to see the results of this type of copy.
These changes have already been implemented twice, and been regressed twice. I'll try once more (with a different approach), then give up ...
As configured, Phpstan running under Php7 reports no errors. However, running under Php8, it reports 100 (!) errors. The vast majority of these are due to two reasons:
- renaming parameters in Php builtin functions in preparation for named parameters.
- using the new class GdImage rather than type resource as the argument type for many image-based functions.
Regardless of the cause, this will be a problem sooner or later. This PR is an attempt to get ahead of that problem. For source members, it mostly adds annotations or updates doc-blocks. Only 2 members have changes to executable code, and these are very minor - BitWise and Writer/Xlsx. For test members, all baseline errors are deleted and the code is fixed. Php7 and Php8 both report no errors with this configuration.
Just reviewing Scrutinizer's list of "bugs". There are 19 ascribed to me. For some, I will definitely take no action (e.g. use of bitwise operators in AND, OR, and XOR functions). However, where I can clean things up so that Scrutinizer is satisfied and the resulting code is not too contorted, I will make an attempt.
This PR corrects 2 problems according to Scrutinizer, and 1 per Phpstan. Only test members are involved.
Specifically, the default for these two functions has been changed from `ENT_COMPAT` to `ENT_QUOTES | ENT_SUBSTITUTE`
This PR configures the argument used for those functions in Settings, and then explicitly applies it everywhere they are used in the codebase.
* Delete Temporary Files In XssVulnerabilityTest
They need not exist after the test. Some of them are placed in
current directory, which means Git thinks they are needed.
* Replace voku/anti-xss with ezyang/htmlpurifier. Despite anti-xss being a smaller footprint dependency, an a better license fit with our MIT license, there are issues with it's automatic it sanitisation of global variables causing side effects
* Additional unit tests for xss in html writer cell comments
This request does not change any source code, only tests.
For a change on which I was working, a test passed when run on its own,
but failed when run as part of the full test suite. It turned out that
an existing test had changed a static value,
thousands separator in this case, and failed to restore it.
The test turned out to be AdvancedBinderTest.
The search for the offending test was more difficult than it should have
been because 26 test scripts which had nothing to do with thousands
separator nevertheless changed that value. They all changed
decimal separator, currency code, and compatibility mode as well,
again for no reason. I changed all of those to eliminate those operations.
I changed the following tests, which actually do change the static
properties identified above for a reason, to restore them as part of teardown.
- CalculationTest sets compatibilityMode and locale
- DayTest sets compatibilityMode, returnDateType, and excelCalendar
- CountTest sets compatibilityMode
- FunctionsTest sets compatibilityMode and returnDateType
- AdvancedValueBinderTest sets currencyCode, decimalSeparator, thousandsSeparator
- StringHelperTest sets currencyCode, decimalSeparator, thousandsSeparator
- NumberFormatTest sets currencyCode, decimalSeparator, thousandsSeparator
- HtmlNumberFormatTest sets currencyCode, decimalSeparator, thousandsSeparator
If row or column dimensions are accessed, then HTML writer would
still generate lots of empty cells, to show nothing at all. This
now ignore row and column dimensions to only output cell that
actually exists (even if those cells are empty).
Fixes#1235Close#1537
Replace default gridlines with different style. Usable in PDF
as well as HTML.
Documentation mentioned use of setUseBOM with Html, but that method
does not exist, and there is no real reason to support it.
Removed it from documentation.
We give users the ability to edit Html/Pdf, but it's a little cumbersome
to use the edited Html for an Html file, and difficult to use it
for a Pdf. I believe we could make it fairly painless in both cases
by allowing the user to set a callback to edit the generated Html.
This can be accomplished with fewer than a dozen lines of very simple code.
I think this would be easier than grabbing the Html in pieces,
editing it, and reassembling it. I think it would also be simpler
than an alternative I considered, namely the addition of a new method
(e.g. saveEditedHtml) to each of the Html and Pdf writers.
One edit that users might like to make when editing html is to add
fallback fonts, something that is not currently available in
PhpSpreadsheet, and might be difficult to add. A natural extension to
that idea would be the use of webfonts, something which is guaranteed
difficult to add. See samples/Basic/17b_Html for an example of this.
None of the PDF writers support webfonts yet. That doesn't mean they
won't do so in future, but, for now, samples/Pdf/21a_Pdf is a prosaic
example of something you could do with this callback. In fact, this
opens the door to letting the user replace the entire body with data
of their choosing, effectively allowing PhpSpreadsheet (where you can
set things like paper size and orientation) to be used as a front-end to
the Pdf processor without the user having to be be overly familiar with
the vagaries of the PDF processor. I think this is actually a pretty
nice idea. YMMV. See samples/Basic/21b_Pdf for an example.
No code changes. The tests in all of these scripts write to at least
one temporary file, which is then read and not used again. The file
should be deleted to avoid filling up the disk system.
This test changes directory then performs an assertion.
No problem if the assertion succeeds. I was a little concerned about
what would happen if the assertion fails, leaving us in the
new directory. So I have changed test to use setUp/tearDown
to ensure that we end up where we started.
There are a number of situations where HTML write was producing
HTML which could not be validated. These include:
- inconsistent use of backslash terminating META, IMG, and COL tags
- @page style tags in body rather than header. Aside from being
non-standard, HTML Reader treats those as spreadsheet data.
- <div style="page-break-before:always" />, a construct which is
usually better handled through css anyhow.
- no alt tag for images (drawings and charts)
Other problems:
- Windows file names not handled correctly for images
- Memory drawings not handled in extendRowsForChartsAndImages
- No handling of different values for showing gridlines
for screen and print
- Mpdf and Dompdf do not require the use of inline css.
Tcpdf remains a holdout in the use of this inferior approach.
- no need to chunk base64 encoding of embedded images
- support for colors in number format was buggy (html tags
run through htmlspecialchars)
Code has been refactored when practical to reduce the number of
very large functions.
Coverage is now 100% for the entire HTML Writer module,
from 75% lines and 39% methods beforehand.
All functions dealing only with charts
are bypassed for coverage because the version of Jpgraph available in
Composer is not suitable for PHP7. The code will, nevertheless,
run successfully, but with warning messages. I have confirmed that
the code is entirely covered, without warnings, when the current
version of Jpgraph is used in lieu of the one available in Composer.
I will be glad to revisit this when the Jpgraph problem is resolved.
Directory PhpSpreadsheetTests/Writer/Html was created to house
the new tests. It seemed logical to move HtmlCommentsTest to
the new directory from PhpSpreadsheetTests/Functional.
A function to generate all the HTML is useful, especially for testing,
but also in lieu of the multiple other generate* functions. I have
added and documented generateHTMLAll.
The documentation for the generate* functions (a) produces invalid html,
(b) produces html which cannot be handled correctly by HTML reader,
and (c) even if those were correct, does not actually affect
the display of the spreadsheet. The documentation has been replaced
by a valid, and more instructive, example.
The (undocumented) useEmbeddedCss property, and the functions
to test and set it are no longer needed. Rather than breaking
existing code by deleting them, I marked the functions deprecated.
This change borrows a change to LocaleFloatsTest from
pull request 1456, submitted a little over a week before this one.
## Improve NumberFormat Support
First phase of this change included correcting NumberFormat handling
in HTML Writer. Certain complex formats could not be handled without
changes to Style/NumberFormat, and I did not wish to combine those changes.
Once the original change had been pushed, I took this part of it back up.
HTML Writer can now handle conditions in formats like:
[Blue][>=3000.5]$#,##0.00;[Red][<0]$#,##0.00;$#,##0.00
In testing, I discovered several errors and omissions
in handling of some other formats.
These are now corrected, and tests added.
oleibman
Owen Leibman
Mark Baker
Adrien Crivelli