-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Fix server processing of an empty data-wp-context
directive
#55482
Fix server processing of an empty data-wp-context
directive
#55482
Conversation
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/experimental/interactivity-api/directives/wp-context.php ❔ phpunit/experimental/interactivity-api/directives/wp-context-test.php |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could be helpful to split the test into two since we're testing two separate situations. In fact, we already have a separate test for malformed JSON, so it could fit nicely to have one for "empty_directive" and one for "directive_without_value"
Either way this is an improvement so it would be good to get in, but it would be nice to split these here or in a follow-up PR.
What do you mean by "empty_directive" and "directive_without_value"? What's the difference between those? |
in the DOM they both carry the value |
Oh, ok. That makes sense, thanks Dennis 🙂 I've added a new test for that use case. I'll wait for your confirmation before merging the PR. |
good to go! |
* Add failing test * Fix the test * Fix WPCS * Add a test for an empty directive
What?
This is a follow up on #55436.
The previous PR fixed the problem when the JSON provided to
data-wp-context
was not valid, and this PR fixes the problem when thedata-wp-context
is empty.Why?
Because, as explained by @dmsnell in #55368, the user might forget the enter any value:
How?
I changed the logic a little bit to accommodate both checks (invalid JSON and empty value).
Testing Instructions
This is covered by a new unit test, but if you want to test it out:
data-wp-context
directive and valid JSON.data-wp-context
.