[wp-trac] [WordPress Trac] #63891: Processing directives function in interactivity API class may cause fatal error

[WordPress Trac]

#63891: Processing directives function in interactivity API class may cause fatal
error
--------------------------------------+--------------------------------
 Reporter:  hugosolar                 |       Owner:  (none)
     Type:  defect (bug)              |      Status:  new
 Priority:  normal                    |   Milestone:  Awaiting Review
Component:  Interactivity API         |     Version:  6.8.2
 Severity:  normal                    |  Resolution:
 Keywords:  has-patch has-unit-tests  |     Focuses:  php-compatibility
--------------------------------------+--------------------------------

Comment (by jonsurrell):

 Checking for `null` as proposed in [https://github.com/WordPress
 /wordpress-develop/pull/9641 PR 9641] is a good idea. I have some other
 thoughts as this is related to HTML API behavior.

 -----

 The behavior may be exceptional with `</br>`, I don't believe there are
 other cases where the tag processor will report an open tag and ''not''
 return an array of attributes.

 This stems from how `</br>` is a bit unusual. It appears to be a close
 tag, but produces a `BR` element with no attributes (`</br class="c">`
 does ''not'' have a class attribute).

 [https://core.trac.wordpress.org/browser/tags/6.8.2/src/wp-includes/html-
 api/class-wp-html-tag-processor.php#L3287 This is handled in]
 `WP_HTML_Tag_Processor::is_tag_closer()`, but `::
 get_attribute_names_with_prefix()`
 [https://core.trac.wordpress.org/browser/tags/6.8.2/src/wp-includes/html-
 api/class-wp-html-tag-processor.php#L2776 checks is_closing_tag directly.]

 {{{#!php
 <?php
 public function get_attribute_names_with_prefix( $prefix ): ?array {
         if (
                 self::STATE_MATCHED_TAG !== $this->parser_state ||
                 $this->is_closing_tag
         ) {
                 return null;
         }

         $comparable = strtolower( $prefix );

         $matches = array();
         foreach ( array_keys( $this->attributes ) as $attr_name ) {
                 if ( str_starts_with( $attr_name, $comparable ) ) {
                         $matches[] = $attr_name;
                 }
         }
         return $matches;
 }
 }}}


 The behavior around `</br>` is somewhat inconsistent in that regard.

 `::get_attribute_names_with_prefix()` could return an empty array in this
 case, making it consistent with other open tags. Swapping
 `$this->is_closing_tag` for `$this->is_tag_closer()` in the snippet above
 seems sufficient in that regard.

 Other methods like `::set_attribute()` continue to check
 `$this->is_closing_tag` and always return `false`. HTML modification
 methods would require more involved HTML changes (eliminate the closing
 `/` from `</br>`), the type of modification that the HTML API has avoided
 so far although it doesn't seem unreasonable.

 [https://github.com/WordPress/wordpress-develop/pull/9657 I've proposed PR
 9657 to address some of these issues.]

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/63891#comment:3>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform