Why is this needed? I thought we had previously decided to omit any tags within the admin bar anyway as part of OD_HTML_Tag_Processor itself.

See this explanation in the PR description:

Lastly, the handling of the admin bar is now consistent with the handling of NOSCRIPT. While no tag visitor will be explicitly be invoked with either the NOSCRIPT tag or any tag in the admin bar, it is possible for tag visitors to manually walk over those tags if they want to. This was done so that the next_tag() method can be (eventually) unmodified in its behavior from the method in the base class.

This will make it easier to switch to WP_HTML_Processor as well, since the next_tag() method won't need to be overridden for that class either.

While no tag visitor will be explicitly be invoked with either the NOSCRIPT tag or any tag in the admin bar, it is possible for tag visitors to manually walk over those tags if they want to.

How would that be possible? Can you share an example?

Yes, for example, in #1746 is about undoing JS-based lazy loading in favor of native lazy loading. Consider this markup used by lazysizes:

<img src="data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7" data-src="https://example.com/bar.jpg" data-srcset="https://example.com/bar-large.jpg 1000w, https://example.com/bar-large.jpg 1000w" sizes="(max-width: 556px) 100vw, 556px" alt="Bar" class="attachment-large size-large wp-image-2 has-transparency lazyload" width="500" height="300">
<noscript>
	<img src="https://example.com/bar.jpg" srcset="https://example.com/bar-large.jpg 1000w, https://example.com/bar-large.jpg 1000w" sizes="(max-width: 556px) 100vw, 556px" alt="Bar" class="attachment-large size-large wp-image-2 has-transparency lazyload" width="500" height="300">
</noscript>

To optimize this, a tag visitor would need to (1) replace the src with the data-src, (2) replace the srcset with the data-srcset, and finally (3) remove the noscript entirely. A tag visitor implementing this could look like the following:

<?php
add_action( 'od_register_tag_visitors', function ( OD_Tag_Visitor_Registry $registry ) {
	$registry->register( 'native-lazy-loading', function ( OD_Tag_Visitor_Context $context ) {
		$processor = $context->processor;
		if ( $processor->get_tag() !== 'IMG' || ! $processor->has_class( 'lazyload' ) ) {
			return; // Tag is not relevant for this tag visitor.
		}

		$src    = $processor->get_attribute( 'data-src' );
		$srcset = $processor->get_attribute( 'data-srcset' );

		if ( ! is_string( $src ) && ! is_string( $srcset ) ) {
			return;
		}

		// Replace the src attribute with the data-src attribute.
		if ( is_string( $src ) ) {
			$processor->set_attribute( 'src', $src );
			$processor->remove_attribute( 'data-src' );
		}

		// Replace the srcset attribute with the data-srcset attribute.
		if ( is_string( $srcset ) ) {
			$processor->set_attribute( 'srcset', $srcset );
			$processor->remove_attribute( 'data-srcset' );
		}

		$processor->remove_class( 'lazyload' );

		if ( $processor->next_tag() && $processor->get_tag() === 'NOSCRIPT' ) {
			if ( method_exists( $processor, 'remove_tag' ) ) {
				// Note: This is not yet supported by the HTML API! But this is how the NOSCRIPT could in theory be removed in the future.
				$processor->remove_tag();
			} elseif ( $processor->next_tag() && $processor->get_tag() === 'IMG' ) {
				// This alternative prevents the duplicate NOSCRIPT > IMG from being shown.
				$processor->remove_attribute( 'src' ); // Prevent loading the image.
				$processor->remove_attribute( 'srcset' ); // Prevent loading the image.
				$processor->set_attribute( 'hidden', true ); // Prevent the image from being rendered twice.
			}
		}
	} );
} );

So in this case, while tag visitors don't iterate over NOSCRIPT elements or their descendants by default, it is useful for there to still be the possibility to do so during their own iteration outside of the overall "optimization loop".

Hmm maybe I'm not understanding what you mean. My question was related to an admin bar example, not noscript.

I've filed an enhancement request in Core-63020 to expose the necessary information in WP_HTML_Processor to be able to construct the XPath in the future without the need for subclassing.

Method marked as private in 2f97645.

That works for now. With the eventual migration to WP_HTML_Processor though, how would you propose to solve this then?

I looked at the 63020 ticket, but don't see how it would address what you flagged above:

then it would need to compute the XPath for every single tag when iterating over the document, and this is more expensive than calling the is_admin_bar() method

Wouldn't we need a method that reads directly from the $open_stack_tags property? For example, something like this:

public function is_within_tag( string $tagname, array $attributes = array() ): bool {
	foreach ( $this->open_stack_tags as $index => $current_tagname ) {
		if ( strtoupper( $tagname ) !== $current_tagname ) {
			continue;
		}
		if ( count( $attributes ) === 0 ) {
			return true;
		}
		if ( ! isset( $this->open_stack_attributes[ $index ] ) ) {
			continue;
		}
		$current_attributes = $this->open_stack_attributes[ $index ];
		if ( $current_attributes === $attributes ) {
			return true;
		}
	}
	return false;
}

Of course, this could become expensive if used excessively or to check for nodes that are deeply nested. So alternatively we could have a method like is_within_root_tag( string $tagname, array $attributes = array() ): bool that does the same as above, but limits it to only checking the first child element of the BODY instead of iterating through all open stack tags.

Just some ideas to consider.

I just updated the description of Core-63020 to make my proposal more explicit. Namely, if there was something like a WP_HTML_Processor::get_element_breadcrumbs() then you could check to see if you are inside of the Admin Bar with this code, similar to what you suggest with is_within_root_tag:

function in_admin_bar( WP_HTML_Processor $processor ): bool {
	$root = $processor->get_element_breadcrumbs()[2] ?? null;
	return (
		null !== $root
		&&
		$root->get_tag() === 'DIV'
		&&
		$root->get_attribute( 'id' ) === 'wpadminbar'
	);
}

That makes sense, thanks for clarifying!