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

Block Library: Add 'object-position' to allowed inline style attributes list #30243

Merged
merged 2 commits into from
Apr 2, 2021

Conversation

Mamaduka
Copy link
Member

Description

The object-position property, used by the Cover block, isn't in the list of allowed CSS attributes. It's removed from the content when non-admin users save a post.

Fixes #29907.

How has this been tested?

  1. Create a post with a cover block.
  2. Edit it as an Author user.
  3. Update cover block's focal point.
  4. Save and refresh the page.
  5. Editor shouldn't display the cover block in recovery mode.

Types of changes

Bugfix

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@Mamaduka Mamaduka added [Type] Bug An existing feature does not function as intended [Block] Cover Affects the Cover Block - used to display content laid over a background image labels Mar 25, 2021
@Mamaduka Mamaduka self-assigned this Mar 25, 2021
@Mamaduka Mamaduka added the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Mar 26, 2021
@Mamaduka
Copy link
Member Author

Mamaduka commented Apr 2, 2021

Core trac ticket: https://core.trac.wordpress.org/ticket/52961.

Copy link
Member

@aristath aristath left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Note: When this gets merged backported to core, object-position should simply be added in the array of allowed attributes inside the safecss_filter_attr function instead.

@Mamaduka
Copy link
Member Author

Mamaduka commented Apr 2, 2021

Thanks, @aristath, for the review.

Note: When this gets merged backported to core, object-position should simply be added in the array of allowed attributes inside the safecss_filter_attr function instead.

Right, I also create PR for it WordPress/wordpress-develop#1156.

My question is if we should keep fallback in the plugin while supporting a non backported core version.

@aristath
Copy link
Member

aristath commented Apr 2, 2021

Right, I also create PR for it WordPress/wordpress-develop#1156.

Awesome 👍

My question is if we should keep fallback in the plugin while supporting a non backported core version.

I'd say yes. The Gutenberg plugin needs to support previous versions of WP and not just the latest one, so this will need to be there.
I would suggest adding a comment in the function stating that it should be removed when the minimum required version is >= 5.8.
We could also add a check and instead of $attrs[] = 'object-position'; do

if ( ! in_array( 'object-option', $attrs ) ) {
    $attrs[] = 'object-position';
}

or instead of return $attrs; do return array_unique( $attrs );, but that may be a bit overkill, there's no real harm in having a duplicate value in that array

@Mamaduka Mamaduka force-pushed the fix/add-safe-style-attrs-filter branch from 2307e11 to ed3d1ca Compare April 2, 2021 07:06
@Mamaduka
Copy link
Member Author

Mamaduka commented Apr 2, 2021

there's no real harm in having a duplicate value in that array

Agree. Added note about removal.

I'll merge this if it's okay with you.

@aristath
Copy link
Member

aristath commented Apr 2, 2021

Agree. Added note about removal.

Thank you :)

I'll merge this if it's okay with you.

Agreed

@Mamaduka Mamaduka merged commit 4b557cb into WordPress:trunk Apr 2, 2021
@github-actions github-actions bot added this to the Gutenberg 10.4 milestone Apr 2, 2021
@Mamaduka Mamaduka deleted the fix/add-safe-style-attrs-filter branch April 2, 2021 08:49
@Mamaduka
Copy link
Member Author

Mamaduka commented Apr 3, 2021

Changes are merged into the core. Changeset https://core.trac.wordpress.org/changeset/50649

@noisysocks noisysocks removed the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Apr 6, 2021
@gziolo gziolo changed the title Add 'object-position' to allowed inline style attributes list Block Library: Add 'object-position' to allowed inline style attributes list Apr 6, 2021
@gziolo gziolo added the [Package] Block library /packages/block-library label Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Cover Affects the Cover Block - used to display content laid over a background image [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.

Non-Admins end up in loop recovering cover block after focal point picker
4 participants