Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Do not serialize scalar values #129

Merged

Conversation

richardhj
Copy link
Contributor

I don't know why all properties are becoming json_encoded, but this resulted in properties having malicious characters (i.e., ") when using asText().

Example:

Before:

$page->getProperty('E-Mail-Adresse')->asText(); // "me@example.org"

After:

$page->getProperty('E-Mail-Adresse')->asText(); // me@example.org

@what-the-diff
Copy link

what-the-diff bot commented Apr 9, 2023

PR Summary

  • Improved asText() method handling
    Added a condition to better handle different content types, improving overall functionality.

@johguentner johguentner added the tests-required Tests for this PR are required label Apr 10, 2023
@johguentner
Copy link
Member

Thank you @richardhj for the PR!
Yeah, this is a valid point! When I wrote the ->asText() method, I wasn't sure what to do with certain properties. For scalar values, this definitely makes sense!

Could you please write tests for the implementation. Then we will merge this within 1.1.0.

@johguentner johguentner added this to the 🍵 v1.1.0 milestone Apr 10, 2023
@johguentner johguentner changed the base branch from dev to fix/as-text-for-scalar-values April 30, 2023 13:46
@johguentner johguentner merged commit 0c0233f into 5am-code:fix/as-text-for-scalar-values Apr 30, 2023
@johguentner johguentner removed the tests-required Tests for this PR are required label Jun 16, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants