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

Several blocks with the anchor are broken in the update #48232

Closed
t-hamano opened this issue Feb 19, 2023 · 11 comments · Fixed by #51288
Closed

Several blocks with the anchor are broken in the update #48232

t-hamano opened this issue Feb 19, 2023 · 11 comments · Fixed by #51288
Assignees
Labels
Backwards Compatibility Issues or PRs that impact backwards compatability [Priority] High Used to indicate top priority items that need quick attention [Type] Bug An existing feature does not function as intended

Comments

@t-hamano
Copy link
Contributor

t-hamano commented Feb 19, 2023

In WordPress 6.1.1, if the content contains blocks with the anchor, updating to WordPress 6.2 Beta2 will break some blocks.

I created the following content in WordPress 6.1.1:

<!-- wp:paragraph -->
<p>Paragraph without anchor</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p id="anchor">Paragraph with anchor</p>
<!-- /wp:paragraph -->

<!-- wp:heading -->
<h2>Heading without anchor</h2>
<!-- /wp:heading -->

<!-- wp:heading -->
<h2 id="anchor">Heading with anchor</h2>
<!-- /wp:heading -->

<!-- wp:group {"layout":{"type":"constrained"}} -->
<div id="anchor" class="wp-block-group"></div>
<!-- /wp:group -->

6 1

If you update this environment to WordPress 6.2, the markup will change as follows:

<!-- wp:paragraph -->
<p>Paragraph without anchor</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p><p id="anchor">Paragraph with anchor</p></p>
<!-- /wp:paragraph -->

<!-- wp:heading -->
<h2 class="wp-block-heading">Heading without anchor</h2>
<!-- /wp:heading -->

<!-- wp:heading {"anchor":"anchor"} -->
<h2 class="wp-block-heading" id="anchor">Heading with anchor</h2>
<!-- /wp:heading -->

<!-- wp:group {"layout":{"type":"constrained"}} /-->

A comparison of the markups confirms the following three points:

  • The paragraph block with the anchor are nested in the p element
  • A comment about anchor attributes has been added to the heading block
  • The anchor in the group block are missing

Now save the post, updating only the title, and reload the browser. Then the block will be broken as follows:

6 2

The browser console displays the following error:

react_devtools_backend.js:4012 Block validation: Block validation failed for `core/paragraph` ({name: 'core/paragraph', icon: {…}, keywords: Array(1), attributes: {…}, providesContext: {…}, …}).

Content generated by `save` function:

<p></p>

Content retrieved from post body:

<p><p id="anchor">Paragraph with anchor</p></p>

---------------

react_devtools_backend.js:4012 Block validation: Block validation failed for `core/group` ({name: 'core/group', icon: {…}, keywords: Array(4), attributes: {…}, providesContext: {…}, …}).

Content generated by `save` function:

<div class="wp-block-group"></div>

Content retrieved from post body:
@t-hamano t-hamano added [Type] Bug An existing feature does not function as intended Backwards Compatibility Issues or PRs that impact backwards compatability labels Feb 19, 2023
@t-hamano t-hamano added the [Priority] High Used to indicate top priority items that need quick attention label Feb 19, 2023
@t-hamano
Copy link
Contributor Author

This issue might be related to #44771.

  • In the Gutenberg repository, start wp-env with WordPress 6.1.1.
  • Disable the Gutenberg plugin.
  • Insert paragraphs, headings, group blocks, etc. with the anchor into the post content.
  • Enable the Gutenberg plugin.
  • When you open the post in the code editor, the markup has changed.
  • Comment out this line added by Add anchor support for dynamic blocks #44771.
  • Open the code editor and confirm that the markup has not changed.

As mentioned above, I think that the changes made in #44771 might have unintentionally affected static blocks.

@t-hamano t-hamano changed the title [WP6.2] Several blocks with anchors are broken in the update [WP6.2] Several blocks with the anchor are broken in the update Feb 19, 2023
@Mamaduka
Copy link
Member

Cc @Soean

@Soean
Copy link
Member

Soean commented Feb 20, 2023

Maybe we should check, if the block has a render callback.

@t-hamano
Copy link
Contributor Author

t-hamano commented Feb 20, 2023

I think the render callback is not limited to dynamic blocks but is also used to rewrite output on the front end. For example, A static block with a render callback will have extra comments added as follows:

<!-- From -->

<!-- wp:heading -->
<h2 id="my-heading">Test</h2>
<!-- /wp:heading -->

<!-- wp:image {"sizeSlug":"large"} -->
<figure class="wp-block-image size-large" id="my-image"><img src="https://live.staticflickr.com/4572/38004374914_6b686d708e_b.jpg" alt=""/></figure>
<!-- /wp:image -->

<!-- To -->

<!-- wp:heading {"anchor":"my-heading"} -->
<h2 class="wp-block-heading" id="my-heading">Test</h2>
<!-- /wp:heading -->

<!-- wp:image {"sizeSlug":"large","anchor":"my-image"} -->
<figure class="wp-block-image size-large" id="my-image"><img src="https://live.staticflickr.com/4572/38004374914_6b686d708e_b.jpg" alt=""/></figure>
<!-- /wp:image -->

I think we should check if the save property of registerBlockType() is null, but the server side cannot determine that...

@annezazu annezazu moved this from ❓ Triage to 🗣️ In discussion, needs decision in WordPress 6.2 Editor Tasks Feb 20, 2023
@annezazu annezazu moved this from 🗣️ In discussion, needs decision to 🏗️ In Progress in WordPress 6.2 Editor Tasks Feb 20, 2023
@t-hamano
Copy link
Contributor Author

As far as I know, I think there is no way for PHP to determine if the block needs to save the anchor attribute as a comment. Therefore, I propose the following approach.

  • Explicitly define the anchor attribute in the dynamic block's block.json
  • Add the following description to the block.json schema and documentation

    For the dynamic block that doesn't save any markup with the save function, it is necessary to define the anchor property with type string as attributes.

@t-hamano t-hamano changed the title [WP6.2] Several blocks with the anchor are broken in the update Several blocks with the anchor are broken in the update Mar 2, 2023
@t-hamano
Copy link
Contributor Author

t-hamano commented Mar 2, 2023

The dynamic block anchor support that caused this problem has been reverted in WordPress 6.2 (See this comment).

This problem needs to be fixed when re-adding anchor support for dynamic blocks in the future. Therefore, move to "Punted to 6.3" on the project board.

@t-hamano
Copy link
Contributor Author

Update: This issue is being attempted to be fixed in #48438.

@ocean90
Copy link
Member

ocean90 commented Apr 25, 2023

I think the changes should have been reverted from the plugin too since this is about losing all the content in a group block with an anchor set. The block recovery won't help here either.
The missing content is also only visible once reloading the editor which is unexpected and thus might get unnoticed.

What are the next steps?

@ndiego ndiego moved this to ❓ Triage in WordPress 6.3.x Editor Tasks May 23, 2023
@ndiego ndiego moved this from ❓ Triage to 🏗️ In Progress in WordPress 6.3.x Editor Tasks May 23, 2023
@t-hamano
Copy link
Contributor Author

t-hamano commented Jun 2, 2023

Update: I have left a comment on how this issue should be addressed in the PR that will correct this problem.

@t-hamano
Copy link
Contributor Author

Update: As mentioned in this comment, I have submitted #51288 to remove anchor support for dynamic blocks in order to find a more ideal solution.

After this PR is merged, I would like to close this issue and submit a new issue to consider re-adding anchor support for dynamic blocks.

@t-hamano
Copy link
Contributor Author

Merged #51288 which revertss anchor support for dynamic blocks. A new issue exploring how to re-add anchor support is #51402.

@priethor priethor removed the [Status] In Progress Tracking issues with work in progress label Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backwards Compatibility Issues or PRs that impact backwards compatability [Priority] High Used to indicate top priority items that need quick attention [Type] Bug An existing feature does not function as intended
Projects
No open projects
5 participants