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

Non-Admins end up in loop recovering cover block after focal point picker #29907

Closed
broksonic21 opened this issue Mar 16, 2021 · 5 comments · Fixed by #30243
Closed

Non-Admins end up in loop recovering cover block after focal point picker #29907

broksonic21 opened this issue Mar 16, 2021 · 5 comments · Fixed by #30243
Assignees
Labels
[Block] Cover Affects the Cover Block - used to display content laid over a background image Needs Testing Needs further testing to be confirmed. [Status] In Progress Tracking issues with work in progress

Comments

@broksonic21
Copy link

Description

We used the cover block in Wordpress 5.6. Upon upgrade to 5.7 and trying to edit the page, you are asked to attempt block recovery. This works fine for admins, but not for non-admins (in our case, Shop Managers (via WooCommerce)).

Step-by-step reproduction instructions

  • Have a page with Cover block in Wordpress 5.6
  • Upgrade to Wordpress 5.7
  • Try editing page as a Shop Manager and see it lets you recover, but then refresh page and see you need to again
  • Try editing page as admin - and see it lets you recover, but then refresh page and see you are good to go and not need to recover again

Expected behaviour

Key other roles can recover or an error is called out as to why not.

Actual behaviour

Shop managers look like they can recover but fail

WordPress information

  • WordPress version: 5.6 and 5.7
  • Gutenberg version: Not Installed
  • Are all plugins except Gutenberg deactivated? No (also have woocommerce)
  • Are you using a default theme (e.g. Twenty Twenty-One)? Storefront
@broksonic21
Copy link
Author

This also causes the focal point picker selection to not be usable by non-admins as well.

@christofervas
Copy link

I can confirm the issue. This happens for non-admins (for example Author role) after changing the "Focal point picker" setting for cover block.

@youknowriad youknowriad added Needs Testing Needs further testing to be confirmed. [Block] Cover Affects the Cover Block - used to display content laid over a background image labels Mar 18, 2021
@talldan
Copy link
Contributor

talldan commented Mar 18, 2021

I've tentatively added this to the 5.7.x must haves project so it doesn't get forgotten, but it does still need testing and confirmation by a dev.

From the report sounds like separately there might be a bug in the block deprecation for the cover block (though I thought they had all been fixed).

@broksonic21
Copy link
Author

To confirm, our content writer ran into this repeatedly, and I agree: The repro is:

  • Have a page with Cover block in Wordpress 5.6
  • Upgrade to Wordpress 5.7
  • Try editing page as a Shop Manager (or Author) and see it lets you recover
  • Try using focal point picker
  • Save
  • Refresh page editor
  • See that you have to recover again

Doing same as admin works w/o repeated recovering.

@broksonic21 broksonic21 changed the title Non-Admins end up in loop recovering cover block Non-Admins end up in loop recovering cover block after focal point picker Mar 18, 2021
@Mamaduka
Copy link
Member

The issue seems to be with the object-position CSS property (added via #25171). It's not in the list of allowed CSS attributes.

When non-admin recovers the block and saves it. The object-position attribute is stripped away, and we end up in recovery mode again.

Adding the following filter should resolve the issue.

add_filter( 'safe_style_css', function( $styles ) {
    $styles[] = 'object-position';
    
    return $styles;
} );

Happy to work on PRs for the plugin and core as well.

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 Needs Testing Needs further testing to be confirmed. [Status] In Progress Tracking issues with work in progress
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants