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

Auto-save performed when manipulating a footnotes block breaks attributes in other blocks. #53212

Closed
celtislab opened this issue Aug 1, 2023 · 6 comments · Fixed by #53257
Closed
Assignees
Labels
[Block] Footnotes Affects the Footnotes Block [Type] Bug An existing feature does not function as intended

Comments

@celtislab
Copy link

Description

In function updateAttributes(attributes) in const updateFootnotes called in the footnote block,

Recursive calls to the updateAttributes function

        if (Array.isArray(value)) {
          attributes[key] = value.map(updateAttributes);
          continue;
        }

If there is an attribute of an array of strings at this time

                imgSizes : {
                    type: "array",
                    items: {
                        type: "string"
                    },
                    default: []
                },

For example, the string "1024x768" is converted to {0: "1", 1: "0", 2: "2", 3: "4", 4: "x", 5: "7", 6: "6", 7: "8"} and an error occurs

Step-by-step reproduction instructions

This happens when you manipulate the footnote block in the editor.

This happens in blocks that use an array of strings in the attributes, but does not seem to be used in the core block.

Screenshots, screen recording, code snippet

gutenberg-footnotes-attribute

Environment info

WordPress 6.3 RC2

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

Yes

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

No

@Mamaduka Mamaduka added Needs Testing Needs further testing to be confirmed. [Block] Footnotes Affects the Footnotes Block labels Aug 1, 2023
@Mamaduka Mamaduka moved this to ❓ Triage in WordPress 6.3.x Editor Tasks Aug 1, 2023
@tellthemachines
Copy link
Contributor

@celtislab could you please provide clear, step-by-step reproduction instructions and a description of what the unexpected behaviour is?

@tellthemachines tellthemachines added the [Status] Needs More Info Follow-up required in order to be actionable. label Aug 1, 2023
@celtislab
Copy link
Author

Only the array of strings and the corresponding function are extracted for reproduction.

    let attr = [];
    attr.sizes = ["1024x768","640x480"];    
    updateAttributes( attr );
    
    function updateAttributes(attributes) {
      attributes = { ...attributes
      };

      for (const key in attributes) {
        const value = attributes[key];

        if (Array.isArray(value)) {
          attributes[key] = value.map(updateAttributes);
          continue;
        }

        if (typeof value !== 'string') {
          continue;
        }

        if (value.indexOf('data-fn') === -1) {
          continue;
        } // When we store rich text values, this would no longer
        // require a regex.


        const regex = /(<sup[^>]+data-fn="([^"]+)"[^>]*><a[^>]*>)[\d*]*<\/a><\/sup>/g;
        attributes[key] = value.replace(regex, (match, opening, fnId) => {
          const index = newOrder.indexOf(fnId);
          return `${opening}${index + 1}</a></sup>`;
        });
        const compatRegex = /<a[^>]+data-fn="([^"]+)"[^>]*>\*<\/a>/g;
        attributes[key] = attributes[key].replace(compatRegex, (match, fnId) => {
          const index = newOrder.indexOf(fnId);
          return `<sup data-fn="${fnId}" class="fn"><a href="#${fnId}" id="${fnId}-link">${index + 1}</a></sup>`;
        });
      }

      return attributes;
    }    

If you do this, it will be called recursively since it is an array.

The "1024x768" string is set as an attribute on recursive calls

However, key becomes undefined and is treated as an object instead of a string, and the converted value is stored as {0: "1", 1: "0", 2: "2", 3: "4", 4: "x", 5: "7", 6: "6", 7: "8"}.

Therefore, if key is undefined, it is likely to need to be handled so that it is not mistakenly converted to an object type.

@ndiego
Copy link
Member

ndiego commented Aug 1, 2023

@celtislab just wanted to confirm the issue here.

So you have a custom attribute that is an array of strings. Given the changes made to handle Footnotes, now, when the attributes are updated, the strings in the array are converted into objects. Is that correct?

@jordesign jordesign added the [Type] Bug An existing feature does not function as intended label Aug 2, 2023
@celtislab
Copy link
Author

That's right.
If you have a block that uses an array of string attributes, please respond quickly, as it will be affected if you use a footnote block!

@andrewserong
Copy link
Contributor

Thanks for reporting this one! I can reproduce too, using a custom block created via using create-block (npx @wordpress/create-block my-test-block) and then adding the following to the newly created block:

In block.json add the following to attributes:

	"attributes": {
		"items": {
			"type": "array",
			"items": {
				"type": "string"
			},
			"default": [ "apples", "oranges" ]
		}
	},

Then, in the block's edit.js file, add the following to output the array of strings within the edit view:

export default function Edit( { attributes } ) {
	const { items } = attributes;
	console.log( items );
	return (
		<div { ...useBlockProps() }>
			{ items.map( ( item, index ) => {
				return <p key={ index }>{ item }</p>
			} ) }
		</div>
	);
}

The expected result:

image

Here's what happens if we add a Paragraph block to the post alongside the custom block, and attempt to add a footnote:

2023-08-02.15.11.55.mp4

From the looks of it, I think we might need some checks at the beginning of updateAttributes within updateFootnotes before the rest expansion of attributes (which converts the string to keys in an object). I'll put up a PR so we can test out a potential fix 🙂

@andrewserong andrewserong moved this from ❓ Triage to 🏗️ In Progress in WordPress 6.3.x Editor Tasks Aug 2, 2023
@andrewserong andrewserong removed [Status] Needs More Info Follow-up required in order to be actionable. Needs Testing Needs further testing to be confirmed. labels Aug 2, 2023
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Aug 2, 2023
@andrewserong
Copy link
Contributor

Potential fix up over in #53257

@andrewserong andrewserong moved this from 🏗️ In Progress to 🔎 Needs Review in WordPress 6.3.x Editor Tasks Aug 2, 2023
@ndiego ndiego moved this from 🔎 Needs Review to 🏗️ In Progress in WordPress 6.3.x Editor Tasks Aug 2, 2023
@github-project-automation github-project-automation bot moved this from 🏗️ In Progress to ✅ Done in WordPress 6.3.x Editor Tasks Aug 2, 2023
@andrewserong andrewserong removed the [Status] In Progress Tracking issues with work in progress label Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Footnotes Affects the Footnotes Block [Type] Bug An existing feature does not function as intended
Projects
No open projects
6 participants