[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