Make WordPress Core

Opened 11 months ago

Last modified 4 months ago

#60826 new defect (bug)

Fix incorrect use of add_theme_support( 'editor-style(s)' )

Reported by: soean's profile Soean Owned by:
Milestone: Future Release Priority: normal
Severity: minor Version:
Component: Themes Keywords: reporter-feedback dev-feedback needs-patch
Focuses: docs Cc:

Description

In the functions add_editor_style(), remove_editor_styles() and remove_theme_support() the Theme $feature parametereditor-style is used instead of editor-styles.

It should be: add_theme_support( 'editor-styles' );

See docs: https://developer.wordpress.org/block-editor/how-to-guides/themes/theme-support/#editor-styles
See code docs: https://github.com/WordPress/wordpress-develop/blob/8d0aed455b1790c5d51386f7675d9e8d68e48edf/src/wp-includes/theme.php#L2672

Change History (17)

This ticket was mentioned in PR #6308 on WordPress/wordpress-develop by @Soean.


11 months ago
#1

  • Keywords has-patch added

@audrasjb commented on PR #6308:


11 months ago
#2

This will require a Unit test update, too.

@Soean commented on PR #6308:


11 months ago
#4

@audrasjb I updated the tests and added a new test for editor styles.

#5 @sabernhardt
11 months ago

Related/duplicate: #51755

#6 @SergeyBiryukov
10 months ago

  • Milestone changed from Awaiting Review to 6.6

#7 @oglekler
8 months ago

I wonder if this can possibly break something for people who used 'editor-style' because it is used in the core? 🤔

add_editor_style() function declaring support for 'editor-style' and I found this confusing, so I added "add_theme_support( 'editor-styles' )" before add_editor_style() in theme I am working on (classic approach works for me better).

I also wonder if test should count on different state for classic and block themes, because for block themes, 'editor-styles' declared by default.

I think, the patch needs rebase.

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


8 months ago

#9 @hellofromTonya
8 months ago

  • Keywords reporter-feedback dev-feedback added

Is the block editor using these functions?

These functions are intended and are documented for the TinyMCE (see [13441] and [16721]).

For example, add_editor_style() is documented as:

Adds callback for custom TinyMCE editor stylesheets.

I'm curious what the impact might be of changing these instances and if any shims might be needed for older and non-block themes using them.

#10 @oglekler
8 months ago

  • Milestone changed from 6.6 to 6.7

Because we need further investigation and clearly a way forward, I am moving this to the next milestone.

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


4 months ago

@khoipro commented on PR #6308:


4 months ago
#12

Hi @Soean for your contribution. Here is some of my feedbacks:

  • A dispatch to work with old version, like TinyMCE (mentioned in Trac)
  • Update related document change (not seeing any there)

#13 follow-up: @chaion07
4 months ago

Thanks @Soean for reporting this. We reviewed this Ticket during a recent bug-scrub session. Here's a summary of the feedback received:

  1. We believe that it requires a dispatch to work with old version, like TinyMCE as mentioned.
  2. Also a document change would be super helpful which we seem to be missing in there.

Thanks.

Props to @khoipro

Cheers!

#14 in reply to: ↑ 13 @azaozz
4 months ago

  • Focuses docs added
  • Keywords needs-patch added; has-patch removed
  • Severity changed from normal to minor

Replying to chaion07:

I'm curious what the impact might be of changing these instances and if any shims might be needed

a document change would be super helpful

Right, this seems like something that can/should be fixed in the documentation, not by changing code as it seems it may introduce backwards compatibility problems.

#15 follow-up: @peterwilsoncc
4 months ago

@azaozz Is it worht aliasing them in the add|remove|has functions in case theme authors have copied the wrong example from WordPress Core?

#16 in reply to: ↑ 15 @azaozz
4 months ago

Replying to peterwilsoncc:

Is it worht aliasing them in the add|remove|has functions in case theme authors have copied the wrong example

Frankly don't think aliasing them is a good idea. If somebody copied the wrong name from somewhere, their code won't work. So they probably debugged it and fixed it.

If they didn't fix it, their code is still not working. Adding an alias will fix this, but then the code that hasn't been working will suddenly start to work. That may be unexpected and seems may cause problems in production/for the users of such themes.

Seems fixing the docs will be best for everybody, and won't cause any problems :)

Perhaps a good idea to help developers that may make this error would be to look for the wrong name(s) and add a doing_it_wrong but only in development mode, so existing production code is not affected.

Last edited 4 months ago by azaozz (previous) (diff)

#17 @desrosj
4 months ago

  • Milestone changed from 6.7 to Future Release

This needs more discussion and the current PR does not seem correct. Punting with 6.7 RC1 due out any moment.

Note: See TracTickets for help on using tickets.