Skip to content

Conversation

@technghiath
Copy link

This is:

- [x] a bugfix
- [ ] a new feature

Checklist:

Why this change is needed?

Add Condition When Read Property On Null
Related issue: #2677

@oleibman
Copy link
Collaborator

Can you add a unit test which would fail without your change, but succeed with it?

oleibman added a commit to oleibman/PhpSpreadsheet that referenced this pull request Sep 22, 2022
Fix PHPOffice#2677. This PR supersedes PHPOffice#2679, written by @technghiath, which lacks tests, and probably doesn't solve the problem entirely. The code causing the problem appears to be the last remnant in Xlsx Reader which calls `children` using a namespace prefix rather than a namespace. That is changed, and tests are added where the tag is unexpectedly missing, and also where it uses a non-standard namespace prefix.
@oleibman oleibman marked this pull request as draft September 22, 2022 05:23
@oleibman
Copy link
Collaborator

Converting to draft. Superseded by #3078. If that is merged, I will close this ticket.

oleibman added a commit that referenced this pull request Sep 28, 2022
* Xlsx Reader External Data Validations Flag Missing

Fix #2677. This PR supersedes #2679, written by @technghiath, which lacks tests, and probably doesn't solve the problem entirely. The code causing the problem appears to be the last remnant in Xlsx Reader which calls `children` using a namespace prefix rather than a namespace. That is changed, and tests are added where the tag is unexpectedly missing, and also where it uses a non-standard namespace prefix.

* Scrutinizer

Reports 1 "new" error. It isn't, but fix it anyhow.

* Fix One Existing Scrutinizer Problem

Only remaining problem in Reader/Xlsx.
@oleibman
Copy link
Collaborator

Superseded by 3078 which was just merged. Closing.

@oleibman oleibman closed this Sep 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants