Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Button block: Fix fatal error on WordPress 6.4 #62730

Closed
wants to merge 7 commits into from

Conversation

cbravobernal
Copy link
Contributor

@cbravobernal cbravobernal commented Jun 21, 2024

What?

For at least one month, Gutenberg should be compatible with WordPress 6.4

A block bindings commit added a render PHP for the button block. And is using a WP_HTML_Tag_Processor method that doesn't exist in 6.4.

6.5 compatibility Tag processor contains it.

Screenshots or screencast

Without the fix:
Screenshot 2024-06-21 at 11 58 59

With the fix:
Screenshot 2024-06-21 at 11 59 24

@cbravobernal cbravobernal added [Type] Bug An existing feature does not function as intended [Package] Block library /packages/block-library labels Jun 21, 2024
Copy link

github-actions bot commented Jun 21, 2024

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: cbravobernal <[email protected]>
Co-authored-by: gziolo <[email protected]>
Co-authored-by: SantosGuillamot <[email protected]>
Co-authored-by: dmsnell <[email protected]>
Co-authored-by: talldan <[email protected]>

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

@gziolo
Copy link
Member

gziolo commented Jun 21, 2024

If there was a fatal error, then it means that no site can currently run Gutenberg with WordPress 6.4, so we should simply bump the minimum required version to WP 6.5.

@cbravobernal
Copy link
Contributor Author

If there was a fatal error, then it means that no site can currently run Gutenberg with WordPress 6.4, so we should simply bump the minimum required version to WP 6.5.

It has been 2 weeks since that error and nobody complaint, so I guess that bumping the version is a quick win.

@SantosGuillamot
Copy link
Contributor

SantosGuillamot commented Jun 21, 2024

It "only" happens when there is a button on the page, but I assume that's a pretty common scenario. Bumping the minimum required version seems logical to me.

Apart from that, with the current implementation, if I have an empty button, it won't remove it in 6.4, right? Which is still a downgrade from the implementation previous to that commit.

Anyway, for me, it's fine to add this check and remove it once 6.6 is out and 6.4 is not supported anymore. Whatever you decide.

@cbravobernal
Copy link
Contributor Author

Apart from that, with the current implementation, if I have an empty button, it won't remove it in 6.4, right? Which is still a downgrade from the implementation previous to that commit.

Yes, as you mention, an empty button will appear.

Screenshot 2024-06-21 at 14 54 22

But still, it would be similar to what the user has in their editor screen, having parity between them.

Screenshot 2024-06-21 at 14 57 45

Has the "removing the button if empty" approach been discussed with design?

@SantosGuillamot
Copy link
Contributor

Has the "removing the button if empty" approach been discussed with design?

I think that logic was added as a result of this issue: link.

@dmsnell
Copy link
Member

dmsnell commented Jun 24, 2024

I would assume that if we rely on that code we would want to or need to backport the Core changes into Gutenberg. That's probably my fault for not keeping up with that.

We would copy the Tag Processor class into Gutenberg and call it Gutenberg_HTML_Tag_Processor_6_5. We could then rely on its methods.

On the other hand we're free to wait until we're two versions ahead of the new methods so we don't have to do any backfilling. Either way: patience, or porting, should be fine.

@SantosGuillamot
Copy link
Contributor

It seems this has been solved in this other pull request, right?

@talldan
Copy link
Contributor

talldan commented Jul 4, 2024

Yes, sorry, I didn't realize this one existed.

@cbravobernal
Copy link
Contributor Author

Let's close this one. @dmsnell also updated all HTML functions compat here:
#63089

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Block library /packages/block-library [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants