Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#27601 closed defect (bug) (fixed)

Add a warning when $editor_id in TinyMCE is invalid

Reported by: toscho Owned by: azaozz
Milestone: 3.9 Priority: normal
Severity: normal Version: 3.9
Component: Editor Keywords:
Focuses: Cc:


When the second argument for wp_editor(), the $editor_id, is invalid, the new TinyMCE will render its content as white text on white background, and the markup is plain text. See #26778.

This change is not backwards-compatible to 3.8, so we should send an explanation to the developer.

That could be either a JavaScript warning per console.warn() (might not work in all browsers) or a PHP notice.

The text could be like this:

Warning: Invalid value for $editor_id. Use alphanumeric characters, underscores and minus only, the first character must not be a number. To set a different name attribute, use the textarea_name parameter in the editor settings array.

Attachments (2)

27601.diff (612 bytes) - added by nacin 5 years ago.
27601.1.diff (633 bytes) - added by azaozz 5 years ago.

Download all attachments as: .zip

Change History (13)

#1 @nacin
5 years ago

  • Milestone changed from Awaiting Review to 3.9
  • Type changed from enhancement to defect (bug)

In the process, we should probably set tinymce to false, reverting to the textarea or text editor. Then things are at least usable. It isn't entirely clear in #26778 whether things are usable at all or not in 3.8, though.

Yes, I agree we should issue some kind of notice. Keeping it in PHP is probably fine.

I imagine the color: white part is to hide contents while TinyMCE is still loading, but I really don't know.

#2 @cconover
5 years ago

Having experienced this problem with one of my plugins, this is only applicable to WordPress 3.9. My plugin had no issues with rendering the editor and working with its contents up through 3.8.1. I like the idea that we should revert to the plain textarea to at least make it usable, but this needs to be accompanied by a notice to the developer.

#3 @nacin
5 years ago

  • Owner set to azaozz
  • Status changed from new to assigned

cconover, are you saying it worked 100% for you in 3.8?

#4 @cconover
5 years ago

nacin, yes it did. I had zero issues prior to testing in any of the 3.9 releases.

#5 @tollmanz
5 years ago

Using editor ID's with brackets did not work 100% in 3.8.1. As I mentioned in 26778, wpLink.open breaks as it uses the editor ID as part of a jQuery selector. I am not sure if this happens in other functions, but I am certain that there was at least one issue with using brackets. I found that everything works so much better if I use alphanumeric values for the ID and set whatever I want for the textarea_name value.

#6 follow-up: @toscho
5 years ago

It was at least possible to read and write text. I am not sure about the link tool.

Currently, you have to know where the problem comes from to solve it. When the back-end was XHTML, you could find it with a validator. But now, in HTML 5, an id attribute with brackets (and other characters) is valid.

Last edited 5 years ago by toscho (previous) (diff)

5 years ago

#7 @nacin
5 years ago

27601.diff is a rough untested proof of concept patch.

5 years ago

#8 @azaozz
5 years ago

27601.diff looks good. The notice is shown right above the editor (27601.1.diff swaps the args order). Wondering if it would be more helpful if instead of __METHOD__ we pass 'WP_Editors::editor()' or 'wp_editor()'.

Last edited 5 years ago by azaozz (previous) (diff)

#9 @nacin
5 years ago

Do we need to set self::$this_quicktags to true?

I agree, we should pass 'wp_editor()' manually.

#10 in reply to: ↑ 6 @azaozz
5 years ago

Without Quicktags and with TinyMCE disabled, wp_editor() would output only the wrapper and the textarea. Still possible to use.

#11 @azaozz
5 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 27950:

Throw a user notice when the editor ID used for TinyMCE contains brackets, props nacin, fixes #27601

Note: See TracTickets for help on using tickets.