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.

Sign up for GitHub

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

Jump to bottom

Html Reader change all zeros to empty string #2810

Closed
1 of 8 tasks
adrienprioul opened this issue May 6, 2022 · 2 comments · Fixed by #2813
Closed
1 of 8 tasks

Html Reader change all zeros to empty string #2810

adrienprioul opened this issue May 6, 2022 · 2 comments · Fixed by #2813

Comments

@adrienprioul
Copy link

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?

Html Reader should read document without altering it (or at least make it configurable)

What is the current behavior?

Html Reader will replace all '0' by empty cell

What are the steps to reproduce?

<?php

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

$reader = IOFactory::createReader('Html');
$spreadsheet = $reader->loadFromString('<html><body><table><tr><td>10</td><td>0</td></tr></table></body></html>');
$sheet = $spreadsheet->getActiveSheet();
echo 'A1 (expected 10): ', $sheet->getCell('A1')->getValue(), PHP_EOL; // A1 (expected 10): 10
echo 'A2 (expected 0): ', $sheet->getCell('A2')->getValue(), PHP_EOL; // A2 (expected 0):

It's caused by this PR imho. '0' being a falsy value code like $child->nodeValue ?: '' make it an empty string.

However I don't quite understand why this change was necessary so I cannot propose a solution right now.

What features do you think are causing the issue

  • Reader
  • Writer
  • Styles
  • Data Validations
  • Formula Calulations
  • Charts
  • AutoFilter
  • Form Elements

Which versions of PhpSpreadsheet and PHP are affected?

1.23
@oleibman
Copy link
Collaborator

oleibman commented May 6, 2022

Your analysis is correct - operator should have been ?? rather than ?:. Working on change, need to add some test cases.

oleibman added a commit to oleibman/PhpSpreadsheet that referenced this issue May 7, 2022
@oleibman
Html Reader Converting Cell Containing 0 to Null String
d533478 
Fix PHPOffice#2810. Repairing some Phpstan diagnostics, used `?:` rather than `??` in a few places.

2 different Html modules are affected. Also, Ods Reader, but its problem is with sheet title rather than cell contents. And, as it turns out, Ods Reader was already not handling sheets with a title of `0` correctly - it made a truthy test before setting sheet title. That is now changed to truthy or numeric. Other readers are not susceptible to this problem. Tests are added.
@oleibman oleibman mentioned this issue May 7, 2022
Merged
7 tasks
@adrienprioul
Copy link
Author

Thank you for your answer and the PR, will be watching for this to be released.

oleibman added a commit that referenced this issue May 10, 2022
@oleibman
Html Reader Converting Cell Containing 0 to Null String (#2813)
070bc68 
Fix #2810. Repairing some Phpstan diagnostics, used `?:` rather than `??` in a few places.

2 different Html modules are affected. Also, Ods Reader, but its problem is with sheet title rather than cell contents. And, as it turns out, Ods Reader was already not handling sheets with a title of `0` correctly - it made a truthy test before setting sheet title. That is now changed to truthy or numeric. Other readers are not susceptible to this problem. Tests are added.
MarkBaker added a commit that referenced this issue Jul 9, 2022
@MarkBaker
1.24.0 - 2022-07-09
ebe8745 
Note that this will be the last 1.x branch release before the 2.x release. We will maintain both branches in parallel for a time; but users are requested to update to version 2.0 once that is fully available.

### Added

- Added `removeComment()` method for Worksheet [PR #2875](https://github.com/PHPOffice/PhpSpreadsheet/pull/2875/files)
- Add point size option for scatter charts [Issue #2298](#2298) [PR #2801](#2801)
- Basic support for Xlsx reading/writing Chart Sheets [PR #2830](#2830)

  Note that a ChartSheet is still only written as a normal Worksheet containing a single chart, not as an actual ChartSheet.

- Added Worksheet visibility in Ods Reader [PR #2851](#2851) and Gnumeric Reader [PR #2853](#2853)
- Added Worksheet visibility in Ods Writer [PR #2850](#2850)
- Allow Csv Reader to treat string as contents of file [Issue #1285](#1285) [PR #2792](#2792)
- Allow Csv Reader to store null string rather than leave cell empty [Issue #2840](#2840) [PR #2842](#2842)
- Provide new Worksheet methods to identify if a row or column is "empty", making allowance for different definitions of "empty":
  - Treat rows/columns containing no cell records as empty (default)
  - Treat cells containing a null value as empty
  - Treat cells containing an empty string as empty

### Changed

- Modify `rangeBoundaries()`, `rangeDimension()` and `getRangeBoundaries()` Coordinate methods to work with row/column ranges as well as with cell ranges and cells [PR #2926](#2926)
- Better enforcement of value modification to match specified datatype when using `setValueExplicit()`
- Relax validation of merge cells to allow merge for a single cell reference [Issue #2776](#2776)
- Memory and speed improvements, particularly for the Cell Collection, and the Writers.

  See [the Discussion section on github](#2821) for details of performance across versions
- Improved performance for removing rows/columns from a worksheet

### Deprecated

- Nothing

### Removed

- Nothing

### Fixed

- Xls Reader resolving absolute named ranges to relative ranges [Issue #2826](#2826) [PR #2827](#2827)
- Null value handling in the Excel Math/Trig PRODUCT() function [Issue #2833](#2833) [PR #2834](#2834)
- Invalid Print Area defined in Xlsx corrupts internal storage of print area [Issue #2848](#2848) [PR #2849](#2849)
- Time interval formatting [Issue #2768](#2768) [PR #2772](#2772)
- Copy from Xls(x) to Html/Pdf loses drawings [PR #2788](#2788)
- Html Reader converting cell containing 0 to null string [Issue #2810](#2810) [PR #2813](#2813)
- Many fixes for Charts, especially, but not limited to, Scatter, Bubble, and Surface charts. [Issue #2762](#2762) [Issue #2299](#2299) [Issue #2700](#2700) [Issue #2817](#2817) [Issue #2763](#2763) [Issue #2219](#2219) [Issue #2863](#2863) [PR #2828](#2828) [PR #2841](#2841) [PR #2846](#2846) [PR #2852](#2852) [PR #2856](#2856) [PR #2865](#2865) [PR #2872](#2872) [PR #2879](#2879) [PR #2898](#2898) [PR #2906](#2906) [PR #2922](#2922) [PR #2923](#2923)
- Adjust both coordinates for two-cell anchors when rows/columns are added/deleted. [Issue #2908](#2908) [PR #2909](#2909)
- Keep calculated string results below 32K. [PR #2921](#2921)
- Filter out illegal Unicode char values FFFE/FFFF. [Issue #2897](#2897) [PR #2910](#2910)
- Better handling of REF errors and propagation of all errors in Calculation engine. [PR #2902](#2902)
- Calculating Engine regexp for Column/Row references when there are multiple quoted worksheet references in the formula [Issue #2874](#2874) [PR #2899](#2899)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Html Reader Converting Cell Containing 0 to Null String oleibman/PhpSpreadsheet
2 participants
@adrienprioul @oleibman