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

Formatting general cell #2239

Closed
sjefke09 opened this issue Jul 27, 2021 · 6 comments
Closed

Formatting general cell #2239

sjefke09 opened this issue Jul 27, 2021 · 6 comments

Comments

@sjefke09
Copy link

sjefke09 commented Jul 27, 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?

Do not format general cells.

What is the current behavior?

In some files, a cell with the format 'General' is formatted.

What are the steps to reproduce?

Some excel files, have named the format 'GENERAL' instead of 'General'. Because the compare in PhpSpreadsheet is case sensitive, it will be formatted. As result, the value is 'GENERAL' instead of the real cell value.
This occurs when the files are created with Apache OpenOffice instead of Microsoft Excel.

Current code [PhpSpreadsheet\Style\NumberFormat\Formatter.php on line 111]:

// For 'General' format code, we just pass the value although this is not entirely the way Excel does it,
// it seems to round numbers to a total of 10 digits.
if (($format === NumberFormat::FORMAT_GENERAL) || ($format === NumberFormat::FORMAT_TEXT)) {
     return $value;
}

Could be (working for us):

// For 'General' format code, we just pass the value although this is not entirely the way Excel does it,
// it seems to round numbers to a total of 10 digits.
if (strtolower($format) === strtolower(NumberFormat::FORMAT_GENERAL) || strtolower($format) === strtolower(NumberFormat::FORMAT_TEXT)) {
    return $value;
}

The same code could be found on PhpSpreadsheet\Style\NumberFormat\Formatter.php on line 168.
There might be some other places where this bug occurs (or maybe also with other formatting options?).

Which versions of PhpSpreadsheet and PHP are affected?

v1.18.0

@oleibman
Copy link
Collaborator

Can you upload a file that demonstrates this problem? I've used Apache OpenOffice to save a file, but I don't see GENERAL, in any configuration of lower/upper case,

@sjefke09
Copy link
Author

passe partouts.xls
Sure, should have done this right away ;)

@oleibman
Copy link
Collaborator

I've downloaded your file. I don't see what the problem is. I was able to read it in using PhpSpreadsheet, and write it out (as both xlsx and xls) without seeing any difference between those and the original. What am I missing?

@sjefke09
Copy link
Author

sjefke09 commented Jul 28, 2021

Did you open it first? Because when you open the file and save it, the problem is (most of the times) gone.
If you use the original file and use the toArray method for sheet 1, some values will be 'GENERAL' instead of the real value it should be. This is the code I use:

$excelFile = \PhpOffice\PhpSpreadsheet\IOFactory::load($filePath);
$sheet = $excelFile->getSheetByName('Blad1');
$array = $sheet->toArray();

echo $array[1][4];

The result should be '21', but I get 'GENERAL' as result.

@oleibman
Copy link
Collaborator

I do duplicate that result. Interesting. getValue and getCalculatedValue are returning the correct result, but getFormattedValue and toArray are not.

oleibman added a commit to oleibman/PhpSpreadsheet that referenced this issue Jul 28, 2021
See issue PHPOffice#2239. Problem is dealt with at the source, by making sure that Reader Xls checks for use of 'GENERAL' rather than 'General'. There doesn't seem to be a reason to test in other places, or to test for other casing variants.
oleibman added a commit that referenced this issue Aug 8, 2021
…ral (#2242)

See issue #2239. Problem is dealt with at the source, by making sure that Reader Xls checks for use of 'GENERAL' rather than 'General'. There doesn't seem to be a reason to test in other places, or to test for other casing variants.
@PowerKiKi
Copy link
Member

Fixed in 1.19.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants