[wp-trac] [WordPress Trac] #60575: Refactor: `data_wp_context` function does not follow WP standards.
WordPress Trac
noreply at wordpress.org
Sun Feb 25 19:30:38 UTC 2024
#60575: Refactor: `data_wp_context` function does not follow WP standards.
--------------------------------------+---------------------
Reporter: cbravobernal | Owner: (none)
Type: defect (bug) | Status: new
Priority: normal | Milestone: 6.5
Component: Editor | Version: trunk
Severity: normal | Resolution:
Keywords: needs-patch dev-feedback | Focuses:
--------------------------------------+---------------------
Comment (by luisherranz):
It's not that easy.
First, it's not easy to spot, but Pascal's code is wrong. It also seems
like nobody else here noticed.
> users would do `data-wp-context=" . wp_interactivity_encode_context(...)
. "`
The correct code is `data-wp-context=' .
wp_interactivity_encode_context(...) . '`.
It's not Pascal's fault. The problem is not obvious by looking at the
code. It's also not easy to notice and fix once it happens. So if seasoned
developers like us make this mistake, imagine other less experienced
developers.
Second, writing `data-wp-context` manually is not safe.
Here, `$some_value` could contain `'> <script>attack()</script>`:
{{{#!php
<div data-wp-context='{ "someValue": "<?php echo $some_value; ?>" }'>
}}}
Even though this one looks better on the surface, it is also subject to
the same attacks:
{{{#!php
<div data-wp-context='<?php echo wp_json_encode( array( 'someValue' =>
$some_value ) ); ?>'>
}}}
**Developers should not have to learn these quirks. We must provide them
with simple functions that are easy to remember, easy to use, and always
safe**.
That's the approach the HTML API is following, and that's what we should
follow for the Interactivity API as well.
**It's our responsibility** to maximize the chances that developers will
use this helper. For that purpose, we must:
- Educate developers to always use the helper function and never write it
manually.
- Provide them with the best possible developer experience to do so.
Educating alone is not enough. If the developer experience is not great,
developers will tend to forget/avoid the cumbersome/boilerplate, and just
write `data-wp-context` manually, leading to errors and attacks. For that
reason, I think **the developer experience should be the top priority
here**.
The best developer experience possible is always the simplest one. If this
is in the intent of the developer:
{{{#!php
<?php
$context = array(
'someValue' => $some_value,
);
?>
<div
<?php echo get_block_wrapper_attributes(); ?>
data-wp-interactive="myPlugin"
data-wp-context='{ ... }'
>
}}}
The simplest thing we can ask them is to replace their `data-wp-context`
string with a `data_wp_context()` call.
{{{#!php
<?php
$context = array(
'someValue' => $some_value,
);
?>
<div
<?php echo get_block_wrapper_attributes(); ?>
data-wp-interactive="myPlugin"
<?php echo data_wp_context( $context ); ?>
>
}}}
There's nothing extra for them to remember, but to use the
`data_wp_context` PHP function instead of the `data-wp-context` string.
That's it. No new names, no extra syntax, no single/double quotes
problems…
Echoing the result is fine, as that's a pattern they are already using,
and it prevents them from having to remember two different function names,
one for `render.php` and another for `render_callback`.
All other options suggested here are valid, but they are suboptimal in my
opinion.
''By the way, I'm also looking forward to HTML templating (#60229), but we
should not wait for that. We should provide the best possible solution
today. For injecting directives using the HTML Tag Processor, we should
also provide a helper, but that's a separate use case and it should be a
separate conversation.''
--
Ticket URL: <https://core.trac.wordpress.org/ticket/60575#comment:13>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform
More information about the wp-trac
mailing list