Make WordPress Core

Opened 11 years ago

Closed 10 years ago

Last modified 10 years ago

#35877 closed enhancement (fixed)

Advanced List Styles Not Saving in Visual Editor in Multisite for Non-SuperAdmins

Reported by: joenasagrc Owned by: jeremyfelt
Priority: normal Milestone: 4.6
Component: Security Version:
Severity: normal Keywords: has-patch commit
Cc: Focuses: administration, multisite

Description

Referencing WordPress Forums: Advanced List Styles Not Saving in WordPress Multisite for Role Administrator (or Below)
​https://wordpress.org/support/topic/advanced-list-styles-not-saving-in-wordpress-multisite-for-role-administrator

Proposed Solution: Add the list type styles to the allowed list in wp-includes/kses.php.


To reproduce:

Instance of WordPress Multisite, TinyMCE Advanced activated and configured with "List Style Options" activated, Default 2014 Theme activated.

Logged in as user with role Administrator or below.

Create ordered list, apply advanced any advanced list style, e.g., Lower Alpha, confirming HTML is

<ol style="list-style-type: lower-alpha;">

Update page, checking HTML: inline style stripped, confirming HTML is now only:

<ol>

For comparison: Reapplied list styles, then added other inline styles such as text color and background. Example:

<p style="border: 1px solid red;padding: 10px">Custom inline style on a paragraph?</p>
<ol style="list-style-type: lower-alpha;">

Result after save: the inline styles remain. List styles are stripped.

<p style="border: 1px solid red;padding: 10px">Custom inline style on a paragraph?</p>
<ol>

Logged in as SuperAdmin: list inline styles remain after save.

Attachments (3)

Ordered-List-Alpha.jpg​ (20.2 KB ) - added by joenasagrc 11 years ago.
Screenshot of Advanced List Styles
35877.patch​ (2.5 KB ) - added by azaozz 11 years ago.
35877.diff​ (2.9 KB ) - added by mrahmadawais 10 years ago.
KSES: Arrange list of allowed CSS attributes alphabatically.

Download all attachments as: .zip

Change History (23)

@joenasagrc
11 years ago

Screenshot of Advanced List Styles

#1 @joenasagrc
11 years ago

  • Summary Advanced List Styles Not Saving Visual Ain WordPress Multisite for Non-SuperAdmins → Advanced List Styles Not Saving Visual in WordPress Multisite for Non-SuperAdmins

#2 @joenasagrc
11 years ago

  • Summary Advanced List Styles Not Saving Visual in WordPress Multisite for Non-SuperAdmins → Advanced List Styles Not Saving in Visual Editor in Multisite for Non-SuperAdmins

#3 @jeremyfelt
11 years ago

  • Keywords needs-patch good-first-bug added
  • Milestone Awaiting Review → Future Release
  • Type defect (bug) → enhancement
  • Version 4.4.2

Hi @joenasagrc, thanks for the ticket!

I don't think there would be harm in allowing the list-style-type property. In the meantime, the safe_style_css filter can be used to solve the issue temporarily.

@azaozz
11 years ago

#4 @azaozz
11 years ago

  • Component TinyMCE → Security
  • Keywords needs-patch good-first-bug removed

I don't see a reason to block list-style-type either.

In 35877.patch: added list-style-type and made it a bit more readable. Thinking we can consider adding more "safe" CSS property names to that list, especially those commonly used in TinyMCE.

#5 @azaozz
11 years ago

  • Keywords good-first-bug added

#6 @jeremyfelt
11 years ago

  • Milestone Future Release → 4.6

This is a great first bug ticket for someone who wants to dig into some allowed CSS properties a bit. Bumping to 4.6 :)

#7 @mrahmadawais
10 years ago

  • Keywords dev-feedback added

Hey, @joenasagrc !

Thanks for the ticket. Welcome here.

@jeremyfelt Can I add display and flex in the list?

This ticket was mentioned in ​Slack in #core by chriscct7. ​View the logs.


10 years ago

#9 @chriscct7
10 years ago

  • Keywords good-wordcamp-ticket added

This ticket was mentioned in ​Slack in #core-multisite by jeremyfelt. ​View the logs.


10 years ago

This ticket was mentioned in ​Slack in #core by ocean90. ​View the logs.


10 years ago

#12 @ocean90
10 years ago

  • Keywords has-patch commit added; good-first-bug dev-feedback good-wordcamp-ticket removed
  • Owner set to jeremyfelt
  • Status new → assigned

#13 follow-up: @jeremyfelt
10 years ago

We're going to get this into 4.7 with the list-style-type property.

@mrahmadawais I think adding display and flex is probably possible. Would you mind opening a new ticket listing out all of the flex properties that would be necessary?

#14 in reply to: ↑ 13 @mrahmadawais
10 years ago

Replying to jeremyfelt:

@mrahmadawais I think adding display and flex is probably possible. Would you mind opening a new ticket listing out all of the flex properties that would be necessary?

Sure, bookmarking it for now. Will do that for 4.7, works for you?

#15 @jeremyfelt
10 years ago

In 37897:

KSES: Adjust the list of safecss attributes for readability.

Props azaozz.
See #35877.

#16 @jeremyfelt
10 years ago

  • Resolution → fixed
  • Status assigned → closed

In 37898:

KSES: Add list-style-type to the list of allowed CSS attributes.

Props azaozz.
Fixes #35877.

@mrahmadawais
10 years ago

KSES: Arrange list of allowed CSS attributes alphabatically.

#17 follow-up: @mrahmadawais
10 years ago

  • Resolution fixed
  • Status closed → reopened

Hey, @jeremyfelt
Can you commit the latest patch I added? It arranges the list of allowed CSS attributes alphabetically.

#18 @mrahmadawais
10 years ago

@jeremyfelt

I have started working on Flexbox properties for KSES safe CSS attributes for WordPress 4.7 at #37248.

#19 in reply to: ↑ 17 ; follow-up: @azaozz
10 years ago

  • Resolution → fixed
  • Status reopened → closed

Replying to mrahmadawais:

It arranges the list of allowed CSS attributes alphabetically.

Why would you want to arrange CSS alphabetically? In this case it makes it harder to read :)

#20 in reply to: ↑ 19 @mrahmadawais
10 years ago

Replying to azaozz:

Why would you want to arrange CSS alphabetically? In this case it makes it harder to read :)

Coz, it makes it easier to read for me. If I am looking for if the list-style-type attribute is there, I will have to look for anything that starts with letter L and not review all the list to find out there is def. a list-style-type attr which is placed at the very end.

INMHO we cannot go on and arrange these elements in order of their relation to one another because not all CSS properties relate to one another.

Note: See TracTickets for help on using tickets.