Skip to content

Conversation

@oandregal
Copy link
Member

@oandregal oandregal commented Oct 19, 2020

In #26192 we refactored the block supports mechanism. It works by holding a global $current_parsed_blocks reference that it uses to determine the attributes that it needs to add to blocks.

However, when a block has inner blocks, that reference isn't updated so inner blocks end up inheriting the parent's properties (style and classes).

How to test

This is what I've done:

  • In your WordPress environment, patch the wp-includes/class-wp-block.php class with the contents of this PR.
  • Create a post that adds the social links block. Publish.
  • Verify that the individual social links (the li elements) don't have the wp-social-links class attached. It should be present in the parent (ul element).

@oandregal oandregal self-assigned this Oct 19, 2020
@oandregal oandregal added the [Type] Bug An existing feature does not function as intended label Oct 19, 2020
@github-actions
Copy link

github-actions bot commented Oct 19, 2020

Size Change: 0 B

Total Size: 1.2 MB

ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.54 kB 0 B
build/api-fetch/index.js 3.35 kB 0 B
build/autop/index.js 2.72 kB 0 B
build/blob/index.js 668 B 0 B
build/block-directory/index.js 8.6 kB 0 B
build/block-directory/style-rtl.css 943 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/index.js 130 kB 0 B
build/block-editor/style-rtl.css 11 kB 0 B
build/block-editor/style.css 10.9 kB 0 B
build/block-library/editor-rtl.css 8.93 kB 0 B
build/block-library/editor.css 8.93 kB 0 B
build/block-library/index.js 144 kB 0 B
build/block-library/style-rtl.css 7.71 kB 0 B
build/block-library/style.css 7.71 kB 0 B
build/block-library/theme-rtl.css 741 B 0 B
build/block-library/theme.css 741 B 0 B
build/block-serialization-default-parser/index.js 1.77 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 47.6 kB 0 B
build/components/index.js 170 kB 0 B
build/components/style-rtl.css 15.4 kB 0 B
build/components/style.css 15.4 kB 0 B
build/compose/index.js 9.63 kB 0 B
build/core-data/index.js 12.1 kB 0 B
build/data-controls/index.js 684 B 0 B
build/data/index.js 8.63 kB 0 B
build/date/index.js 31.9 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 4.43 kB 0 B
build/edit-navigation/index.js 10.6 kB 0 B
build/edit-navigation/style-rtl.css 868 B 0 B
build/edit-navigation/style.css 871 B 0 B
build/edit-post/index.js 306 kB 0 B
build/edit-post/style-rtl.css 6.37 kB 0 B
build/edit-post/style.css 6.35 kB 0 B
build/edit-site/index.js 21.6 kB 0 B
build/edit-site/style-rtl.css 3.8 kB 0 B
build/edit-site/style.css 3.81 kB 0 B
build/edit-widgets/index.js 26.6 kB 0 B
build/edit-widgets/style-rtl.css 3.09 kB 0 B
build/edit-widgets/style.css 3.09 kB 0 B
build/editor/editor-styles-rtl.css 480 B 0 B
build/editor/editor-styles.css 482 B 0 B
build/editor/index.js 42.5 kB 0 B
build/editor/style-rtl.css 3.85 kB 0 B
build/editor/style.css 3.84 kB 0 B
build/element/index.js 4.45 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.49 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 1.74 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.54 kB 0 B
build/is-shallow-equal/index.js 709 B 0 B
build/keyboard-shortcuts/index.js 2.39 kB 0 B
build/keycodes/index.js 1.85 kB 0 B
build/list-reusable-blocks/index.js 3.02 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.12 kB 0 B
build/notices/index.js 1.69 kB 0 B
build/nux/index.js 3.27 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.44 kB 0 B
build/primitives/index.js 1.35 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/reusable-blocks/index.js 3.06 kB 0 B
build/rich-text/index.js 13 kB 0 B
build/server-side-render/index.js 2.61 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.24 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.75 kB 0 B
build/warning/index.js 1.13 kB 0 B
build/wordcount/index.js 1.23 kB 0 B

compressed-size-action

Copy link
Member

@mkaz mkaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested this out and works as expected. 👍

The change makes sense, make sure the global has the right value before render and replace it with the previous value after.

@oandregal
Copy link
Member Author

oandregal commented Oct 19, 2020

cc @tellthemachines we need this (or an alternative that fixes the bug) for the release. Flagging in case I'm not around when that happens.

@oandregal oandregal added this to the Gutenberg 9.2 milestone Oct 19, 2020
@oandregal oandregal force-pushed the fix/current-parsed-block-inner-blocks branch from f9ce774 to 519a496 Compare October 19, 2020 19:58
@oandregal
Copy link
Member Author

I think the change is solid, but there's a couple of things I don't know how to do:

  • how does the patch work for the plugin while WP core is not updated?
  • how/when we backport these kinds of changes to WP core?

It's a bit late over here so I'll leave the PR unmerged so others can chime in.

@mkaz
Copy link
Member

mkaz commented Oct 19, 2020

@nosolosw I believe the process is to open a core ticket with the diff.

I created a ticket and diff here: https://core.trac.wordpress.org/ticket/51569

@gaambo
Copy link
Contributor

gaambo commented Nov 4, 2020

I think this could have also been solved by looking at #25900 - because if pre_render_block would be called for innerBlocks as well (as I would have expected) than the original code from #26192 would have also worked.

@oandregal
Copy link
Member Author

Thanks for flagging, Fabian. I see there's a core ticket https://core.trac.wordpress.org/ticket/51612 for this as well as a potential solution at WordPress/wordpress-develop#691

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Type] Bug An existing feature does not function as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants