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

Site Editor bug exporting patterns as JSON #55746

Closed
pbiron opened this issue Oct 31, 2023 · 8 comments · Fixed by #55912
Closed

Site Editor bug exporting patterns as JSON #55746

pbiron opened this issue Oct 31, 2023 · 8 comments · Fixed by #55912
Assignees
Labels
[Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced [Type] Bug An existing feature does not function as intended

Comments

@pbiron
Copy link

pbiron commented Oct 31, 2023

Description

Reporting this upstream after investigating Trac ticket 59771.

On the trac ticket to the OP notes that when exporting a pattern in the site editor (w/ 6.4 RC2, no GB plugin) and then trying to import that pattern into another site he gets an "Invalid JSON" error. He also notes that exporting the same pattern from the "Manage all my patterns" list table he is able to successfully import the pattern into another site.

Neither I nor @hellofromtonya could replicate the problem at first. So, I asked the OP to attach his JSON files to the trac ticket. Indeed, the JSON files he exported in the site editor and from the list table where different. After some investigation I determined that the problem is that an em-dash character (codepoint U+2014) that appears in the pattern is not being correctly UTF-8 encoded in the JSON export from the site editor (which makes the JSON invalid), but is being encoded correctly from the list table export.

Note that in the OP's pattern, there are other non-ASCII chars that are being correctly UTF-8 encoded.

After some further investigation, I noticed that the JS that does the JSON export in the 2 cases is different.

  • site editor uses exportAsJSON() in wp-includes/js/dist/edit-site.js
  • list table row action uses exportReusableBlock() in wp-includes/js/dist/list-reusable-blocks.js

Note: In both cases, the source of that JS is somewhere else and those files are generated by webpack. I haven't been able to figure where the un-mangled source JS is...I'm sure someone more familiar with the codebase will know ;-)

While the JS is the 2 cases is slightly different, in looks "equivalent" to me at first glance...basically, doing JSON.stringify() on a JS object, one property of which is post.content.raw. That leads me to think that the problem is in how that post.content.raw is retrieved in the 2 cases.

Step-by-step reproduction instructions

  1. create a pattern that contains an em-dash
  2. go into Editor > Patterns and click the "3 dots" under the pattern created in step 1 and choose "Export as JSON"
  3. go into Editor > Manage all of my patterns, when on the list table click the "Export as JSON" row action (be sure to save the site will a different name than in step 2)
  4. on the Patterns list table screen, click the "Import from JSON" button and import the file exported in step 2 and receive a "Invalid JSON" error
  5. on the Patterns list table screen, click the "Import from JSON" button again and import the file exported in step 3 and receive a success message

Screenshots, screen recording, code snippet

On the trac ticket, the OP linked to videos of him doing the steps in "Step-by-step reproduction instructions" above.

Environment info

WP 6.4-RC2
Windows WAMP-like server setup (PHP 8.1, MySQL 8.0, apache 2.4)
Lastest Windows Chrome, Edge, Firefox

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@hellofromtonya hellofromtonya added [Type] Bug An existing feature does not function as intended [Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced labels Oct 31, 2023
@hellofromtonya
Copy link
Contributor

Thinking this one could be a 6.4.1 candidate, unless there's something that should be fixed for 6.4.0. Pinging the 6.4 Editor Triage Leads for awareness @annezazu @bph

@hellofromtonya
Copy link
Contributor

Also, the OP shared the following to help with triage:

@bph
Copy link
Contributor

bph commented Nov 1, 2023

Totally agree, @hellofromtonya a great candidate for 6.4.1.

Nice sleuthing, @pbiron 👏

@steve-dougherty
Copy link

Hello, as I mentioned when I first reported this, I'm completely new to the bug reporting thing and I'm interested in seeing the entire process if possible. If this post is not allowed here then my apologies and please delete it.
Otherwise,

  1. Is this where I can follow the process to the fix?
  2. Is there a guide that details the process of reporting a bug all the way through to the end of that bug to the fix?

Thank you.

  • Steve D

@pbiron
Copy link
Author

pbiron commented Nov 1, 2023

@steve-dougherty : glad you made it here to follow progress on your bug report.

There are 2 pages in the Core "handbook" that describe the bug reporting process:

Unfortunately, neither of them completely cover the process for an issue like yours, which affects the Site Editor...such issues being handled here in a "Gutenberg GitHub Issue".

That said, just stay tuned to this issue/ticket in GitHub and you'll be able to follow the progress being made on finding a solution to the problem. When an proposed fix is available, it's likely that someone will reach out to you and ask you to test it.

@steve-dougherty
Copy link

@pbiron Thank you very much for your reply. Looking forward to seeing how the sausage is made. That sounded better in my head.

@bph bph moved this from Triage to Punted to 6.4.1 in WordPress 6.4 Editor Tasks Nov 3, 2023
@bph
Copy link
Contributor

bph commented Nov 3, 2023

@steve-dougherty Yes, this issue is a good place to follow up on the progress of the bug resolution. Once a contributor starts a PR (Pull Request) and mentioned this issue there it will show up on this issue's page as well, in both the activity log and the Developement section in the sidebar.
Thank you for noting that we would need to make it a bit more explicity to find out what happened to a bug report on the way to resolution.

@ramonjd
Copy link
Member

ramonjd commented Nov 7, 2023

I think the fix might be to use the same, internal function as the reusable blocks.

I've tested it and it works so far. 🤞🏻

The downloadjs module patterns is using seems to have a non-resolved bug where it cannot properly encode non-ASCII characters.

I ran some things through downloadjs to check it out and, yes, this input:

{ content: "<!-- wp:paragraph -->\n<p>École,Être,Allô  Études—Bonjour!™£¢∞§¶•ªº .</p>\n<!-- /wp:paragraph -->", }

becomes

{ "content": "<!-- wp:paragraph -->\n<p>�cole,�tre,All�  �tudes�Bonjour!"�����"�� .</p>\n<!-- /wp:paragraph -->",}

I'll throw a PR up to remove the library and use the existing function.

@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Nov 7, 2023
@github-project-automation github-project-automation bot moved this from Punted to 6.4.1 to Done in WordPress 6.4 Editor Tasks Nov 7, 2023
@bph bph removed the [Status] In Progress Tracking issues with work in progress label Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced [Type] Bug An existing feature does not function as intended
Projects
No open projects
Status: Done
5 participants