Skip to content
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

Cell style modified #2441

Closed
toboul70 opened this issue Dec 8, 2021 · 6 comments · Fixed by #2444
Closed

Cell style modified #2441

toboul70 opened this issue Dec 8, 2021 · 6 comments · Fixed by #2444

Comments

@toboul70
Copy link

toboul70 commented Dec 8, 2021

This is:

- [x ] a bug report
- [ ] a feature request
- [ ] **not** a usage question (ask them on https://stackoverflow.com/questions/tagged/phpspreadsheet or https://gitter.im/PHPOffice/PhpSpreadsheet)

What is the expected behavior?

Cell Style not modified

What is the current behavior?

Cell Style modified

What are the steps to reproduce?

I created an xlsx model with Excel with cells like that
1

I made a reader.
I modified some cells (not the cell I show)
I made a writer.

I opened the file. The cell style was modified like that
2

File :
Classeur1.xlsx

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:

<?php

require __DIR__ . '/vendor/autoload.php';

// Create new Spreadsheet object
$excel  = new PhpOffice\PhpSpreadsheet\Spreadsheet();
$excel  = PhpOffice\PhpSpreadsheet\IOFactory::load($file);
$org    = $excel->getSheetByName('Sheet1');
$org->setCellValue('A1', 'Hello World');

$writer = PhpOffice\PhpSpreadsheet\IOFactory::createWriter($excel, 'Xlsx');
$writer->save($newfile);

Which versions of PhpSpreadsheet and PHP are affected?

PHP 7.3.21 (cli) (built: Aug 4 2020 11:21:19) ( ZTS MSVC15 (Visual C++ 2017) x64 )
Copyright (c) 1997-2018 The PHP Group
Zend Engine v3.3.21, Copyright (c) 1998-2018 Zend Technologies

"name": "phpoffice/phpspreadsheet",
"version": "1.20.0",
"source": {
"type": "git",
"url": "https://github.com/PHPOffice/PhpSpreadsheet.git",
"reference": "44436f270bb134b4a94670f3d020a85dfa0a3c02"

@toboul70 toboul70 changed the title Bad style Cell style modified Dec 8, 2021
@oleibman
Copy link
Collaborator

oleibman commented Dec 9, 2021

I can tell you what happened. I need to do some research to figure out what should happen. Fill patterns are assigned a start and end color in the constructor. In your original spreadsheet, the fill pattern is output without a start and end color. In the copied version, it is output with a start and end color, which I believe is correct for some other patterns, but apparently not correct for lightUp and, presumably, other patterns.

@toboul70
Copy link
Author

toboul70 commented Dec 9, 2021

My original file
2

The copy file
1

@toboul70
Copy link
Author

toboul70 commented Dec 9, 2021

Another try

My file
1

The copy file
2

In the two cases, the background color is set to grey, just under blank.

@toboul70
Copy link
Author

toboul70 commented Dec 9, 2021

Another example :

A cell style modified by PHPExcel :

$sheet->getFill()->setFillType(Fill::FILL_PATTERN_DARKUP);

3

oleibman added a commit to oleibman/PhpSpreadsheet that referenced this issue Dec 11, 2021
Fix PHPOffice#2441. The Fill constructor sets start color to white and end color to black and the Xlsx writer writes these values to the output file. This appears to be the wrong setting for all 7 LIGHT* pattern types, 2 of the 7 DARK* patterns (DARKGRAY and DARKTRELLIS), and 1 of the 3 GRAY patterns (GRAY0625). When the wrong colors are written at save time, those patterns are not as expected. Xls writer does not appear to have the same problem.

The XML does not require either a start or end color, and the omission of these colors in the file being read was responsible for the problem. The code is changed to mimic that behavior by omitting the color tags at write time if they have not changed from when they were created by the Fill constructor (they will be written for gradient or solid patterns regardless).

This is another change which is easier to confirm via samples rather than tests. There are separate samples for Xlsx and Xls; as Excel will be quick to warn you, Xls is not as fully functional as Xlsx with respect to fill patterns. The samples do include a cell where one of the cells (LightGrid in C11) explicitly specifies the "default" colors.
@lucasres
Copy link

I have similar problem, when set a background color in 1.20.0 version dont fill range cell but works well in 1.17.1

$spreadsheet
  ->getActiveSheet()
  ->getStyle("$range1:$range2")
  ->getFill()
  ->setFillType(Fill::FILL_SOLID)
  ->getStartColor()
  ->setARGB($cellColor);

I dont know if same problem.

Which versions of PhpSpreadsheet and PHP are affected?
PHP 7.4.16 (cli) (built: Apr 1 2021 15:55:43) ( NTS )
Copyright (c) The PHP Group
Zend Engine v3.4.0, Copyright (c) Zend Technologies

"name": "phpoffice/phpspreadsheet",
"version": "1.20.0",
"source": {
"type": "git",
"url": "https://github.com/PHPOffice/PhpSpreadsheet.git",
"reference": "44436f270bb134b4a94670f3d020a85dfa0a3c02"
}

@oleibman
Copy link
Collaborator

I am guessing that you are specifying $cellColor as a 6-hex-character string. Argb needs to be 8 characters, rgb needs to be 6. This was not enforced in 1.17.1 but is enforced in 1.20.0.

oleibman added a commit that referenced this issue Dec 18, 2021
* Fill Pattern Start and End Colors

Fix #2441. The Fill constructor sets start color to white and end color to black and the Xlsx writer writes these values to the output file. This appears to be the wrong setting for all 7 LIGHT* pattern types, 2 of the 7 DARK* patterns (DARKGRAY and DARKTRELLIS), and 1 of the 3 GRAY patterns (GRAY0625). When the wrong colors are written at save time, those patterns are not as expected. Xls writer does not appear to have the same problem.

The XML does not require either a start or end color, and the omission of these colors in the file being read was responsible for the problem. The code is changed to mimic that behavior by omitting the color tags at write time if they have not changed from when they were created by the Fill constructor (they will be written for gradient or solid patterns regardless).

This is another change which is easier to confirm via samples rather than tests. There are separate samples for Xlsx and Xls; as Excel will be quick to warn you, Xls is not as fully functional as Xlsx with respect to fill patterns. The samples do include a cell where one of the cells (LightGrid in C11) explicitly specifies the "default" colors.

* Scrutinizer

It somehow ascribed to me a problem in code which was unchanged by this PR. Correct it anyhow, along with some Phpstan fixes (errors now ignored because of change).

* Added Tests

Also corrected some docBlock problems with Style/*/parent and getSharedComponent.

* Create 2 Abstract Methods

Scrutinizer complained that 2 methods found in all Supervisor sub-types were not defined in Supervisor. Add abstract methods to satisfy it.

* Scrutinizer Ignoring Typehints

Try this instead.

* Slight Improvement

Better handling of Style->getParent().
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

3 participants