-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Duplicating worksheet is not copying styling #2494
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Actually, most of the styling (borders, bold, etc.) is being handled just fine. But, you are right - there are at least 2 problems. First, we not yet support the default Excel styling of hyperlinks. This is more difficult than it seems. The more important problem is the fill colors. That seems to be due to PR #1694. That change was discussed at length in issue #2366. It was felt at the time that the regression cases were due to user coding errors. However, your sample file makes it clear that such a problem can make it into the wild. What is happening is that the Xlsx reader is expecting the fill color to be specified as, say: <fgColor rgb="FFBFBFBF"/> In other words, it is expecting an 8-character string despite the attribute name of <fgColor rgb="BFBFBF" /> The Style Reader tries to set ARGB using this 6-character string, and it fails. I will add a fix for this to PR #2471, which makes extensive changes to the Xlsx Style Reader, and which will remain in draft status for at least a couple of weeks. If you see additional styling differences apart from the 2 discussed above, please let me know. |
This Excel output was generated by the open source library EPPlus - this isn't the first time I've seen issues as a result of how EPPlus outputs its XLSX files. I'll see if I can find other stylings that aren't being copied over properly. |
Fix PHPOffice#2494. Apparently EPPlus outputs fill colors as `<fgColor rgb="BFBFBF">` while most output fill colors as `<fgColor rgb="FFBFBFBF">`. EPPlus actually makes more sense. Regardless, validating length of rgb/argb is a recent development for PhpSpreadsheet, under the assumption that an incorrect length is a user error. This development invalidates that assumption, so restore the previous behavior. In addition, a comment in Colors.php says that the supplied color is "the ARGB value for the colour, or named colour". However, although named colors are accepted, nothing sensible is done with them - they are passed unchanged to the ARGB value, where Excel treats them as black. The routine should either reject the named color, or convert it to the appropriate ARGB value. This change implements the latter.
* WIP Namespacing Phase 2 - Styles This is part 2 of a several-phase process to permit PhpSpreadsheet to handle input Xlsx files which use unexpected namespacing. The first phase, introduced as part of release 1.19.0, essentially handled the reading of data. This phase handles the reading of styles. More phases are planned. It is my intention to leave this in draft status for at least a month. This will give time for additional testing, by me and, I hope, others who might be interested. This fixes the same problem addressed by PR #2458, if it reaches mergeable status before I am ready to take this out of draft status. I do not anticipate any difficult merge conflicts if the other change is merged first. This change is more difficult than I'd hoped. I can't get xpath to work properly with the namespaced style file, even though I don't have difficulties with others. Normally we expect: ```xml <stylesheet xmlns="http://whatever" ... ``` In the namespaced files, we typically see: ```xml <x:stylesheet xmlns:x="http://whatever" ... ``` Simplexml_load_file specifying a namespace handles the two situations the same, as expected. But, for some reason that I cannot figure out, there are significant differences when xpath processes the result. However, I can manipulate the xml if necessary; I'm not proud of doing that, and will gladly accept any suggestions. In the meantime, it seems to work. My major non-standard unit test file had disabled any style-related tests when phase 1 was installed. These are now all enabled. * Scrutinizer Its analysis is wrong, but the "errors" it pointed out are easy to fix. * Eliminate XML Source Manipulation Original solution required XML manipulation to overcome what appears to be an xpath problem. This version replaces xpath with iteration, eliminating the need to manipulate the XML. * Handle Some Edge Cases For example, Style file without a Fills section. * Restore RGB/ARGB Interchangeability Fix #2494. Apparently EPPlus outputs fill colors as `<fgColor rgb="BFBFBF">` while most output fill colors as `<fgColor rgb="FFBFBFBF">`. EPPlus actually makes more sense. Regardless, validating length of rgb/argb is a recent development for PhpSpreadsheet, under the assumption that an incorrect length is a user error. This development invalidates that assumption, so restore the previous behavior. In addition, a comment in Colors.php says that the supplied color is "the ARGB value for the colour, or named colour". However, although named colors are accepted, nothing sensible is done with them - they are passed unchanged to the ARGB value, where Excel treats them as black. The routine should either reject the named color, or convert it to the appropriate ARGB value. This change implements the latter.
This is:
What is the expected behavior?
Expecting a worksheet to be fully duplicated using this code
$clonedWorksheet = clone $spreadsheet->getSheetByName('Worksheet 1');
$clonedWorksheet->setTitle('Copy of Worksheet 1');
$spreadsheet->addSheet($clonedWorksheet);
What is the current behavior?
Some cell styles like fills and font colors are not copied over
What are the steps to reproduce?
Please provide a Minimal, Complete, and Verifiable example of code that exhibits the issue without relying on an external Excel file or a web server:
Which versions of PhpSpreadsheet and PHP are affected?
1.21
Elevation.xlsx
The text was updated successfully, but these errors were encountered: