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

Social Links colors not working for roles without unfiltered_html capabilities #29297

Closed
oandregal opened this issue Feb 24, 2021 · 5 comments · Fixed by #29330
Closed

Social Links colors not working for roles without unfiltered_html capabilities #29297

oandregal opened this issue Feb 24, 2021 · 5 comments · Fixed by #29330
Assignees
Labels
[Block] Social Affects the Social Block - used to display Social Media accounts [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@oandregal
Copy link
Member

oandregal commented Feb 24, 2021

In #28084 we added new color controls to style the icons and their background. The way this works is by using CSS Custom Properties attached to the block (post_content) such as the block style attribute looks like:

style="--wp--social-links--icon-color:#FFFFFF"

This is problematic for kses which maintains a list of allowed property names and --wp--social-links--icon-color is not one of them (see list). As a result, the color controls won't work for roles that don't have unfiltered_html capabilities.

Steps to reproduce:

  • Use a WordPress 5.7 environment (latest from wordpress-develop trunk or using the beta-tester plugin) without the Gutenberg plugin.
  • Log in as an author role (or any other role without unfiltered_html capabilities).
  • Go to the post editor and add a social links block.
  • Select white as the icon color and orange as the background color.
  • Publish and go to the front-end => note that the colors are not present. Inspect the DOM and see that the CSS variables used to set the colors are not attached to the DOM.
  • Reload the editor => the block is invalidated.
@oandregal oandregal added [Block] Social Affects the Social Block - used to display Social Media accounts [Type] Bug An existing feature does not function as intended labels Feb 24, 2021
@oandregal
Copy link
Member Author

oandregal commented Feb 24, 2021

cc @noisysocks so you're aware of this (I've already added it to the 5.7 must-haves). Do I need to create a core patch as well?

cc @aaronrobertshaw @mkaz @jasmussen for ideas.

One option would be to hook into the safe_style_css filter and add --wp--social-links--icon-color and --wp--social-links--icon-background-color as allowed property names. However, I'm inclined to think that when #28913 is ready the social links block won't need to use CSS Custom Properties to pass style values down the tree. Hence, I'm not sure allowing those property names is what we want for the future.

An alternative is to use the block context to pass the values down to the inner blocks.

Finally, we always have the option to revert the original PR.

@mcsf
Copy link
Contributor

mcsf commented Feb 24, 2021

Finally, we always have the option to revert the original PR.

Unfortunately, this is something we may have to resort to if we can't address the filtering issue. @aaronrobertshaw, can you lead the fix?

@mkaz
Copy link
Member

mkaz commented Feb 24, 2021

@nosolosw My thought would be to add the filter now and then we can remove after #28913 goes in.

I wonder if we should look at modifying KSES to allow for all custom CSS properties, it seems like overkill to restrict, though I'm not sure how easy it is to modify or not.

@oandregal
Copy link
Member Author

oandregal commented Feb 24, 2021

I've given this more thought and I've grown reservations against the safe_style_css approach, here's my braindump:

  • I presume it'd work for most of the cases but there may be environments that have stricter kses enforcement. It also sounds like that kind of change would merit wider exposure before merging into WordPress core.

  • There's also the fact that this feature would be used and so CSS vars would be attached to the post_content and a future change will have them removed with the required block deprecations, etc. Once they're shipped, there may be expectations that they are public API so people use them for different purposes and we may not be able to remove them.

Given this, I'm inclined to think that we should refactor to use the block context approach or revert that PR.

@aaronrobertshaw
Copy link
Contributor

@nosolosw I have a PR up passing the colors through block context to the inner social links. If it's not what you were imagining let me know and I'll tweak it as required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Social Affects the Social Block - used to display Social Media accounts [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants