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

Reader\Xlsx - showGridlines option is set when shouldn't #912

Closed
xklid101 opened this issue Feb 27, 2019 · 2 comments · Fixed by #4098
Closed

Reader\Xlsx - showGridlines option is set when shouldn't #912

xklid101 opened this issue Feb 27, 2019 · 2 comments · Fixed by #4098

Comments

@xklid101
Copy link
Contributor

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?

when reading - do not set xml attribute showGridlines from element sheetView to true when xml attribute gridlinesSet from element printOptions is true

What is the current behavior?

when reading - xml attribute showGridlines from element sheetView is set to true when xml attribute gridlinesSet from element printOptions is true

What are the steps to reproduce?

Just read any xlsx file saved in a way that has gridlines invisible for sheetView (showGridLines="false") and contains printOptions element with attribute gridLinesSet="true"

e.g. in sheet1.xml

...
<sheetView showGridLines="false" showRowColHeaders="1" tabSelected="0" workbookViewId="0" zoomScale="55" zoomScaleNormal="55">
    <selection activeCell="B17" sqref="B17"/>
</sheetView>
...
...
<printOptions gridLines="false" gridLinesSet="true" headings="true" horizontalCentered="false" verticalCentered="false"/>
...

after reading this file, the result created with phpspreadsheet will allways have showGridLines="true" in sheetView

and it is because of this part of code in reader (mainly the first "if" block) https://github.com/PHPOffice/PhpSpreadsheet/blob/1.6.0/src/PhpSpreadsheet/Reader/Xlsx.php#L880-L885

and according to some specification i've found, the code highlighted should change into this:

if (self::boolean((string) $xmlSheet->printOptions['gridLines']) && self::boolean((string) $xmlSheet->printOptions['gridLinesSet'])) {
    $docSheet->setPrintGridlines(true);
}

Which versions of PhpSpreadsheet and PHP are affected?

phpspreadsheet 1.6.0
any php version

Specification that i've found can be downloaded here https://www.iso.org/standard/71691.html

and contains

...
gridLines (Print
Grid Lines)
Used in conjunction with gridLinesSet. If both gridLines and gridlinesSet are true , then
grid lines shall print. Otherwise, they shall not (i.e., one or both have false values).
The possible values for this attribute are defined by the W3C XML Schema boolean
datatype.

gridLinesSet (Grid
Lines Set)
Used in conjunction with gridLines. If both gridLines and gridLinesSet are true , then
grid lines shall print. Otherwise, they shall not (i.e., one or both have false values).
The possible v
...

so nothing about sheetView, but only about printing

  • I'm not 100percent sure if I'm right - but it makes sense to me - so if you do not use some other specification for open xml formats for xlsx...
@stale
Copy link

stale bot commented Apr 28, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If this is still an issue for you, please try to help by debugging it further and sharing your results.
Thank you for your contributions.

@stale stale bot added the stale label Apr 28, 2019
@stale stale bot closed this as completed May 5, 2019
@oleibman
Copy link
Collaborator

Reopening. It seems a bit odd, but legitimate. Expect a fix in a day or two.

@oleibman oleibman reopened this Jul 16, 2024
@stale stale bot removed stale labels Jul 16, 2024
oleibman added a commit to oleibman/PhpSpreadsheet that referenced this issue Jul 17, 2024
Fix PHPOffice#912, opened in Feb. 2019, and closed as stale in Apr. 2019, and which I have re-opened to be closed properly by this PR. Another "better late than never". Original issue says that print options should not affect ShowGridlines, which seems true enough. Aside from that, the existing code isn't quite correct anyhow. Excel looks for 2 attributes, one of which must be explicitly set to true and the other of which must not be explicitly set to false, in order to determine whether PrintGridlines should be set. PhpSpreadsheet is changed to do the same. This could be treated as a BC break for the unusual situation described in the issue, but it seems more like a bug fix to me.
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.

2 participants