File Block: Follow HTML API best practices

[Mamaduka]

What?

This is a follow-up #60494 (comment).

PR updates render_block_core_file render callback to follow HTML API best practices.

Why?

See: #60494 (comment)

Testing Instructions

1. Create a post.
2. Add a file block and upload a PDF file.
3. Save the post and preview it.
4. Confirm that the inline preview is rendered correctly.

Testing Instructions for Keyboard

Same.

Screenshots or screencast

[dmsnell]

this and the lines down to 38 below should be equivalent to the following…

// If there are no OBJECT elements, something might have already modified the block.
if ( ! $processor->next_tag( 'OBJECT' ) ) {
	return $content;
}

if I’m understanding correctly, the goal is to find the first OBJECT element, and return the $content if none is found.

the while is going to stop at the first OBJECT opening tag because once it finds one, 'OBJECT' !== $processor->get_tag() will be false unless it failed to find one at all, which is picked up below.

[Mamaduka]

Thanks, @dmsnell!

That makes sense and is more convenient.

[dmsnell]

always worth a check for is_string( $filename ) because boolean attributes carry the non-empty value of true. at some point I think we’ll have something like get_string_attribute() or get_attribute_value() but we don’t have a super-clear picture of the best method at the current time. for now, it’s annoying, but true masquerades the empty string, which in the DOM are equivalent.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
Co-authored-by: dmsnell <dmsnell@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[dmsnell]

something I didn’t notice the first time around is that if we don’t find an OBJECT element, we also don’t set the data-wp-interactive attribute. this may be what we intend, but I wanted to point it out because I see it now

[Mamaduka]

Technically, this shouldn't add any overhead. If OBJECT doesn't exist, we return unmodified content.

I guess we can bookmark the wrapper element, then check the objects' existence and only add attributes if the objects exist. Is there a better way to do it?

[dmsnell]

performance shouldn’t be in question here, only behavior. so for example, if you wanted every file block to get the Interactivity attribute, then that wouldn’t be the case if they contain no OBJECT.

I think that’s intended, so it should be fine, but I wanted to note that as a thing to double-check.

the point is that even if we call set_attribute() we won’t see that applied unless we use or return $processor->get_updated_html(). it is practically “free” to set an attribute and then abandon the processor though.

[Mamaduka]

As far as know it should be only interactive if the File block displays PDF preview, which won’t be the case when object element isn’t present.

If a plugin fully replaces the output then they should handle it, or try and match the core block structure.

I don’t have a strong preference here, if you have a suggestion to improve and showcase the best practices, I would gladly apply it 🙇

[dmsnell]

Looks all good from my vantage point 👍

[Mamaduka]

@dmsnell, thanks for the feedback. Should I merge this?

[dmsnell]

Sorry @Mamaduka — I could have hit approve before. Inasmuch as the HTML API code looks, this is good. I didn’t test any of the changes, assuming you did 👍