[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.5
 Severity:  normal                    |  Resolution:
 Keywords:  has-patch has-unit-tests  |     Focuses:  php-compatibility
--------------------------------------+--------------------------------

Comment (by dmsnell):

 Here are a couple of notes about the history of this usage, as the issue
 at hand lies with the Interactivity API’s use of the HTML API, not with
 the defined and commented and annotated behaviors of the HTML API.

 `::get_attribute_names_with_prefix()` may return `null`, so it’s always
 the case that we need to assume it may do that and write our code
 accordingly. it essentially indicates the difference between //this has no
 attributes// and //this cannot have any attributes in any situation//. So
 for opening tags we find an empty set of arrays, but HTML comments, text
 nodes, etc…, which cannot have attributes — those return `null`. That
 nuance is not well documented in the function interface, and we could also
 go about updating it to be clearer. Regardless, `null` is still a valid
 return type.

 The issue with `</br>` is identical to with an unexpected `</p>`. These
 tags //cannot// contain HTML attributes and so return `null`. We discussed
 the role of the Tag Processor in these cases of whether they should report
 as closing tags or as opening tags and we decided at the time to try and
 keep the Tag Processor focused as a //lexer// and the HTML Processor as
 the machine of imbuing semantic interpretation to the tokens (@zieladam
 was part of these discussions and I can’t remember his thoughts on the
 matter).

 It’s probably fine/safe to consider `</br>` an opening tag because it’s
 atomic, but if we do that it does then raise concerns about inconsistency
 with `</p>` because _sometimes_ the `</p>` is an opening tag and
 _sometimes_ it’s a closing tag, but the Tag Processor lacks the knowledge
 to make that decision. In the same way there are a plethora of cases where
 the low-level tokenization remains a bit less expected //because// the
 nuance is **if you want to know semantic information about the HTML you
 need to use the HTML Processor**. Here are some other examples:

  - in the Tag Processor, `<image>` is an opening tag whose name is
 `IMAGE`. In the HTML Processor it’s a void element whose name is `IMG`,
 unless it’s in the SVG or MathML context and then it’s still an `IMAGE`.
 `get_tag()` reports the name difference.
  - in the Tag Processor, `</sarcasm>` is a closing tag whose name is
 `SARCASM`. In the HTML Processor it’s non-existent; completely skipped-
 over.
  - in the Tag Processor, `</p><p>` contains a closing `P` tag and then an
 opening `P` tag. In the HTML Processor it contains an opening `P` tag, a
 closing `P` tag, a second opening `P` tag, and a second closing `P` tag,
 in that order. the Tag Processor has no way of knowing whether `</p>`
 should open or close a `P` element.

 `</br>` is one of very few exceptions I would feel comfortable
 incorporating into the HTML API, but I am also very reluctant to move
 //some// semantic exceptions into the Tag Processor when we know that
 someone needs to rely on the HTML Processor if they want that. we might
 already make some exceptions like this and this may all seem duplicitous,
 but I want to stress that just because something is unexpected or because
 very-low-level primitives are severely delegated between the two classes,
 that does not imply we necessarily have to change the interface.

 ----

 My recommendation is that code calling `get_attribute_names_with_prefix()`
 which does not check for `null` simply do this for convenience.

 {{{#!php
 <?php
 $count = count( $processor->get_attribute_names_with_prefix() ?? array()
 );
 }}}

 We can step back and note that this reported problem does not manifest
 when using the HTML Processor.

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