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 loads one empty comment for each proper comment in spreadsheet #375

Closed
xklid101 opened this issue Feb 14, 2018 · 5 comments
Closed

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?

For each comment load just one proper comment into sheet

What is the current behavior?

When sheet has some comment, Xlsx reader loads two comments instead of one. The first has correct value and is placed in correct cell. The second one is empty and placed one cell left from proper comment

What are the steps to reproduce?

Create some xlsx file with comment e.g. in D4, load file with Reader\Xlsx and get output. Someone will see 2 comments in sheet - one in D4 (this is ok), and one empty in C4 (this is wrong)

Which versions of PhpSpreadsheet and PHP are affected?

branch develop, PHP 5.6


I believe the bug is here

$comment = $docSheet->getCommentByColumnAndRow((string) $column, $row + 1);

If I remove this:

$comment = $docSheet->getCommentByColumnAndRow((string) $column, $row + 1);

and insert that instead:

$comment = $docSheet->getCommentByColumnAndRow($column + 1, $row + 1);

everything is working and looks ok

@madorin
Copy link

madorin commented Feb 19, 2018

I have the same problem, just tried to update from an earlier beta build to latest stable one.

@PowerKiKi
Copy link
Member

Would one of you be able to submit a PR that would fix the bug and cover it with unit tests ?

@xklid101
Copy link
Contributor Author

xklid101 commented Mar 4, 2018

I'll try to do it asap

@stale
Copy link

stale bot commented May 3, 2018

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 May 3, 2018
@xklid101
Copy link
Contributor Author

xklid101 commented May 4, 2018

Pullrequest to fix this issue is ready to merge (or some comment needed)
#420

@stale stale bot removed the stale label May 4, 2018
Dfred pushed a commit to Dfred/PhpSpreadsheet that referenced this issue Nov 20, 2018
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