WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 12 months ago

Last modified 5 months ago

#19173 closed defect (bug) (maybelater)

Issues with wp_editor() when used inside of a meta box.

Reported by: goto10 Owned by: azaozz
Milestone: Priority: normal
Severity: normal Version: 3.3
Component: Editor Keywords:
Focuses: Cc:

Description

I ran into a couple of obscure issues when adding an editor inside of a meta box. Errors are generated when moving the meta box around and then switching between the Visual and HTML editors. Also, the resize handle (.mceResize) is a bit too low when viewing the Visual tab when our custom editor has been added within a meta box. Tested in FF7 7.0.1 WPv3.3beta2

I'm adding the Editor within a meta box because it's the easiest way to add an Editor field type within the meta box/ CPT plugin I've made. It seems like it will be a common use case.

Here's a simple plugin that adds an editor within a meta box.

<?php
/**
 * Plugin Name: WP Editor in a Meta Box
 * Plugin URI: 
 * Description: Demonstration of WP Editor in a meta box.
 * Version: 1.0
 * Author: goto10
 * Author URI: 
 * License: 
 */

// file name: wp_editor-in-meta-box-test.php 

/* Meta box code based on http://codex.wordpress.org/Function_Reference/add_meta_box */

/* Define the custom box */
add_action( 'add_meta_boxes', 'myplugin_add_custom_box' );

/* Do something with the data entered */
add_action( 'save_post', 'myplugin_save_postdata' );

/* Adds a box to the main column on the Post and Page edit screens */
function myplugin_add_custom_box() {
	add_meta_box( 'wp_editor_test_1_box', 'WP Editor Test #1 Box', 'wp_editor_meta_box' );
}

/* Prints the box content */
function wp_editor_meta_box( $post ) {

  // Use nonce for verification
  wp_nonce_field( plugin_basename( __FILE__ ), 'myplugin_noncename' );
	
	$field_value = get_post_meta( $post->ID, '_wp_editor_test_1', false );
	wp_editor( $field_value[0], '_wp_editor_test_1' );
}

/* When the post is saved, saves our custom data */
function myplugin_save_postdata( $post_id ) {
  
	// verify if this is an auto save routine. 
  // If it is our form has not been submitted, so we dont want to do anything
  if ( defined( 'DOING_AUTOSAVE' ) && DOING_AUTOSAVE ) 
      return;

  // verify this came from the our screen and with proper authorization,
  // because save_post can be triggered at other times
  if ( ( isset ( $_POST['myplugin_noncename'] ) ) && ( ! wp_verify_nonce( $_POST['myplugin_noncename'], plugin_basename( __FILE__ ) ) ) )
      return;

  // Check permissions
  if ( ( isset ( $_POST['post_type'] ) ) && ( 'page' == $_POST['post_type'] )  ) {
    if ( ! current_user_can( 'edit_page', $post_id ) ) {
			return;
		}		
  }
	else {
		if ( ! current_user_can( 'edit_post', $post_id ) ) {
			return;
		}
	}

  // OK, we're authenticated: we need to find and save the data
	if ( isset ( $_POST['_wp_editor_test_1'] ) ) {
		update_post_meta( $post_id, '_wp_editor_test_1', $_POST['_wp_editor_test_1'] );
	}
	
}

Case 1:

Steps to reproduce error:

Activate test plugin.
HTML tab is selected in our custom editor instance.
Drag meta box containing editor around (ensuring the jQuery UI Sortable drop placeholder shows up, but the box does not need to be docked to a new location)
Release the box
Switch to the editor's Visual tab


The following error occurs:

j is null
In file: /wp-includes/js/tinymce/wp-tinymce.php?c=1&ver=345-20110918
The top row of tinymce icons is also duplicated. Once the error occurs, we can no longer switch between tabs.



Case 2:

Steps to reproduce error:

Activate test plugin.
Visual tab selected.
Drag meta box containing editor around.
Release the box.
switch to the editor's HTML tab.


The following error occurs:

Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIDOMHTMLDocument.implementation]
In file: /wp-includes/js/tinymce/wp-tinymce.php?c=1&ver=345-20110918

Note: It may take a couple of tries when switching between visual/HTML tabs to trigger the errors. Sometimes the errors won't happen, but usually they do.

Change History (27)

#1 @ocean90
5 years ago

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

It works in Chrome 17 dev.

Opera error log:

Uncaught exception: TypeError: Cannot convert 'p.getBody()' to object

Error thrown at line 1, column 309502 in <anonymous function: getContent>(q) in http://wpms.dev/wp-includes/js/tinymce/tiny_mce.js?ver=345-20111105:
    r=p.getBody().innerHTML
called from line 1, column 340171 in g() in http://wpms.dev/wp-includes/js/tinymce/tiny_mce.js?ver=345-20111105:
    return b.trim(f.getContent({format:"raw",no_events:1}))
called from line 1, column 340603 in <anonymous function: add>(m) in http://wpms.dev/wp-includes/js/tinymce/tiny_mce.js?ver=345-20111105:
    m.content=g();
called from line 1, column 307310 in <anonymous function: save>(u) in http://wpms.dev/wp-includes/js/tinymce/tiny_mce.js?ver=345-20111105:
    p.undoManager.add()
called from line 1, column 306249 in <anonymous function: hide>() in http://wpms.dev/wp-includes/js/tinymce/tiny_mce.js?ver=345-20111105:
    p.save();
called from line 36, column 4 in <anonymous function: go>(a) in http://wpms.dev/wp-admin/js/editor.dev.js?ver=20110820:
    ed.hide();
called from line 1, column 0 in <anonymous function>(event):
    switchEditors.go(this);

Safari error log:

TypeError: 'undefined' is not an object (evaluating 'j.createRange')

#2 @jakemgold
5 years ago

I'm going to break out the position of the resize handle into a separate ticket, as this is a more granular issue that isn't limited to meta boxes.

#3 @nacin
5 years ago

  • Milestone changed from Awaiting Review to 3.3

I'm going to break out the position of the resize handle into a separate ticket, as this is a more granular issue that isn't limited to meta boxes.

Indeed.

#4 @azaozz
5 years ago

The problem is that TinyMCE, once initialized cannot be moved in the DOM, or rather the browsers cannot handle it being moved. Moving the postbox triggers this. Some browsers/versions handle that better than others as @ocean90 mentions above but generally the editor textarea and iframe shouldn't be moved.

As $goto10 mentions using wp_editor() would be quite common inside a postbox, so we will need to find an easy way to disable moving of that postbox (dragging it on the screen doesn't count as moving although it's very CPU intensive, re-inserting the parent element somewhere else in the DOM is the problem).

Looks like we will need yet another arg on the postboxes to disallow moving but other suggestions are welcome.

Version 0, edited 5 years ago by azaozz (next)

#5 follow-up: @azaozz
5 years ago

Another possibility would be to get the content of the editor and then destroy the instance on dragstart. Then on dragend initialize it again and set the content. Such code should go in postboxes.js inside the UI.sortable, will run some tests with that too.

#6 follow-up: @goto10
5 years ago

...so we will need to find an easy way to disable moving of that postbox...

Looks like we will need yet another arg on the postboxes to disallow moving but other suggestions are welcome.

I think it would be nice to be able to add a meta box that is invisible. It would just show the content that is output by the callback function passed to add_meta_box(). The normal draggable/collapsible meta box stuff would not be there. The content within these boxes would act similarly to the title/permalink and main editor fields - they can't be moved and there is no visible wrapping container.

It seems like this would be a good way of facilitating the requirement of not allowing editors to be moved and, it would follow the existing convention of non movable items established by the title and editor fields.

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

Replying to goto10:

I think it would be nice to be able to add a meta box that is invisible.

Thinking more about that, not sure it's a good idea to have a static (non-movable) metabox in between the "normal" metaboxes.

We have do_action('edit_page_form') or do_action('edit_form_advanced') that can be used to output any html under div#normal-sortables, perhaps these should be used to add extra editors? BTW we probably should pass $post_type to these actions for better control.

Alternatively we could add another action that would output html right under the main editor, i.e. above div#normal-sortables.

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

#8 @goto10
5 years ago

@azaozz

I think it would be nice to be able to add a meta box that is invisible.

Thinking more about that, not sure it's a good idea to have a static (non-movable) metabox in between the "normal" metaboxes.


Yes, you're right. Static boxes intermingled with traditional meta boxes would feel strange.

Typically I have all of my form fields within various meta boxes, and I loop through the array of boxes/fields to show them (in order) on the appropriate post edit screen. I was trying to think of a way that editor fields could still be added by using the same call to add_meta_box() (albeit with an extra parameter for boxes containing an editor).

Thanks for the heads up on the 'edit_page_form' and 'edit_form_advanced' hooks. These sound like good alternatives, though I do think it's important to be able to able place editors exactly where we want to. I guess I just want to have my cake and eat it too :) Maybe your second proposal with saving editor content, destroying the editor, then recreating it would be better then.

#9 @azaozz
5 years ago

  • Milestone 3.3 deleted
  • Resolution set to maybelater
  • Status changed from reviewing to closed

Per the IRC discussion today, wp_editor() shouldn't be used in a metabox. do_action('edit_page_form') or do_action('edit_form_advanced') are the proper places for it.

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

#10 follow-up: @nacin
5 years ago

In fairness, this only applies for wp_editor() with the visual editor enabled. HTML-only should be fine, I imagine.

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

Replying to nacin:

Right. The limitation is that TinyMCE cannot be moved. Quicktags works fine.

#12 follow-up: @goto10
5 years ago

...wp_editor() shouldn't be used in a metabox.

Ah, I see. Thanks for giving it a go and for the tip on being able to use the HTML-only editor within metaboxes. That's pretty cool!

#13 in reply to: ↑ 12 ; follow-up: @azaozz
5 years ago

Replying to goto10:

Ah, I see. Thanks for giving it a go and for the tip on being able to use the HTML-only editor within metaboxes. That's pretty cool!

Yes, looked at this yesterday in IRC and the consensus was not to try finding a workaround for moving TinyMCE in the DOM. Also even if we output a non-movable postbox, it would still move in the DOM when other postboxes are added/removed above or below it, so that's not going to work.

#14 in reply to: ↑ 13 ; follow-up: @JPry
5 years ago

Replying to azaozz:

Yes, looked at this yesterday in IRC and the consensus was not to try finding a workaround for moving TinyMCE in the DOM. Also even if we output a non-movable postbox, it would still move in the DOM when other postboxes are added/removed above or below it, so that's not going to work.

Based on my testing, it appears that the current output for a custom editor when using either do_action('edit_page_form') or do_action('edit_form_advanced') puts the editor in the midst of the custom meta boxes. My assumption is that this means that the additional editors are still being moved in the DOM, but I admit that the DOM and how it changes with javascript is not something I'm really familiar with.

Regardless of this, wouldn't it also make sense to have additional actions before and after the custom meta boxes that could also be hooked into? That can give us better control of where the additional editors are displayed. Of course, if I'm missing something or doing it the wrong way, let me know.

#15 in reply to: ↑ 14 @azaozz
5 years ago

Replying to JPry:

No, do_action('edit_page_form') or do_action('edit_form_advanced') run after the postbox container div and their position in the DOM is static (the editor wrap DIV becomes a sibling of div#normal-sortables and div#advanced-sortables).

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

#16 @simonwheatley
4 years ago

  • Cc simon@… added

#17 in reply to: ↑ 5 ; follow-up: @devesine
4 years ago

Replying to azaozz:

Another possibility would be to get the content of the editor and then destroy the instance on dragstart. Then on dragend initialize it again and set the content. Such code should go in postboxes.js inside the UI.sortable, will run some tests with that too.

(Since we just addressed this, I thought I'd post what information we have.)

We're using the following code to do this in WP 3.5, and it seems to be working well and robustly (with multiple metaboxes containing multiple TinyMCE editors). The selectors could probably be refined a bit.

// TinyMCE doesn't handle being moved in the DOM.  Destroy the
// editor instances at the start of a sort and recreate 
// them afterwards.
var _triggerAllEditors = function(event, creatingEditor) {
	var postbox, textarea;

	postbox = $(event.target);
	textarea = postbox.find('textarea.wp-editor-area');

	textarea.each(function(index, element) {
		var editor;
		editor = tinyMCE.EditorManager.get(element.id);
		if (creatingEditor) {
			if (!editor) {
				tinyMCE.execCommand('mceAddControl', true, element.id);
			}
		}
		else {
			if (editor) {
				editor.save();                                          
				tinyMCE.execCommand('mceRemoveControl', true, element.id);
			}       
		} 
	});
};
$('#poststuff').on('sortstart', function(event) {
	_triggerAllEditors(event, false);
}).on('sortstop', function(event) {
	_triggerAllEditors(event, true);
});

#18 @deanmarktaylor
4 years ago

  • Cc deanmarktaylor added

#19 @sscovil
4 years ago

  • Cc sscovil@… added

#20 in reply to: ↑ 17 @bjorsq
3 years ago

I've tested this, but have encountered errors when the editor is switched to text/html mode using the tabs in the top right. It works great as long as the editor is in tinymce mode, but when it is in text/html mode there are sometimes problems when dragging the metabox, and tinymce toolbars are added when the metabox is dropped (under the bbcode toolbar). I added a check for each editor which looks for the tmce-active class on a parent first, and it seems to work OK - here is the modified _triggerAllEditors function:

	var _triggerAllEditors = function(event, creatingEditor) {
		var postbox, textarea;

		postbox = $(event.target);
		textarea = postbox.find('textarea.wp-editor-area');

		textarea.each(function(index, element) {
			var editor, is_active;

			editor = tinyMCE.EditorManager.get(element.id);
			is_active = $(this).parents('.tmce-active').length;

			if (creatingEditor) {
				if (!editor && is_active) {
					tinyMCE.execCommand('mceAddControl', true, element.id);
				}
			}
			else {
				if (editor && is_active) {
					editor.save();
					tinyMCE.execCommand('mceRemoveControl', true, element.id);
				}       
			} 
		});
	};

Replying to devesine:

Replying to azaozz:

Another possibility would be to get the content of the editor and then destroy the instance on dragstart. Then on dragend initialize it again and set the content. Such code should go in postboxes.js inside the UI.sortable, will run some tests with that too.

(Since we just addressed this, I thought I'd post what information we have.)

We're using the following code to do this in WP 3.5, and it seems to be working well and robustly (with multiple metaboxes containing multiple TinyMCE editors). The selectors could probably be refined a bit.

// TinyMCE doesn't handle being moved in the DOM.  Destroy the
// editor instances at the start of a sort and recreate 
// them afterwards.
var _triggerAllEditors = function(event, creatingEditor) {
	var postbox, textarea;

	postbox = $(event.target);
	textarea = postbox.find('textarea.wp-editor-area');

	textarea.each(function(index, element) {
		var editor;
		editor = tinyMCE.EditorManager.get(element.id);
		if (creatingEditor) {
			if (!editor) {
				tinyMCE.execCommand('mceAddControl', true, element.id);
			}
		}
		else {
			if (editor) {
				editor.save();                                          
				tinyMCE.execCommand('mceRemoveControl', true, element.id);
			}       
		} 
	});
};
$('#poststuff').on('sortstart', function(event) {
	_triggerAllEditors(event, false);
}).on('sortstop', function(event) {
	_triggerAllEditors(event, true);
});

This ticket was mentioned in IRC in #wordpress-dev by ericandrewlewis. View the logs.


3 years ago

#22 follow-up: @pawelkmpt
12 months ago

  • Resolution maybelater deleted
  • Status changed from closed to reopened

I see that 4 years ago you decided to not find workaround for this problem but it's still an issue and I think that maybe there's a chance that you'll look at it once more. Maybe TinyMCE has changed and it's easier to resolve the problem today.

Problem appears in WooCommerce, but as they use wp_editor() I believe it's WP error. So when I'm editing content in Product Short Description using Visual editor and than move meta box to the other place (drag&drop) than:

  • content from visual editor disappears (but it still stays in text editor),
  • it's not possible to edit content in visual editor anymore,
  • I can switch to Text editor in Chrome but cannot do that in Firefox having NS_ERROR_UNEXPECTED: wp-tinymce.php:6:0 error in console

Setup:

  • WordPress 4.3
  • WooCommerce 2.4.6
  • Chrome 45
  • Firefox 40

#23 in reply to: ↑ 22 ; follow-up: @azaozz
12 months ago

Replying to pawelkmpt:

content from visual editor disappears (but it still stays in text editor),
it's not possible to edit content in visual editor anymore

This is a browser limitation. Happens because iframes cannot be moved safely in the DOM. They get reset/reloaded. If you look around, you'll probably find few old bug reports for Firefox and WebKit/Chrome that are closed as "wontfix".

As far as I see there are two options to improve this:

  • "Lock" the container postbox so it cannot be dragged when using the visual editor (preferred). The user will have to "fold" the postbox to be able to drag it.
  • Save the content and destroy the editor instance on dragStart, then initialize the editor again and load the content on dragEnd. This is more complex and will make the dragging somewhat hard as processing and saving the editor content can be slow.

IMHO the plugin that requires visual editor in a draggable container should decide which approach to use. We probably can add support for the first approach to wp_editor() when used in a postbox, but that's not a priority.

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

#24 in reply to: ↑ 23 ; follow-up: @mboynes
12 months ago

  • Safe the content and destroy the editor instance on dragStart, then initialize the editor again and load the content on dragEnd. This is more complex and will make the dragging somewhat hard as processing and saving the editor content can be slow.

I've tried that, and it provides a crummy UX. The editor disappears and leaves an HTML-filled textarea on drag. In Fieldmanager, we reinit the editors after the user drags them. This way, TinyMCE sticks around for the drag, which is much less confusing for users. In terms of performance, this seems to work well (though I've never tried it on an older machine).

It would be nice if there were a more elegant solution, but this is the best I was able to come up with, and it works. To @azaozz's point, this is a solution I implemented on the plugin level, and I didn't think much of needing to do so. I don't think that core necessarily needs to support this behavior, but I also wouldn't oppose it.

#25 in reply to: ↑ 24 @aware
12 months ago

Replying to mboynes:

  • Save the content and destroy the editor instance on dragStart, then initialize the editor again and load the content on dragEnd. This is more complex and will make the dragging somewhat hard as processing and saving the editor content can be slow.

Completely agree. We're doing this now for a plugin we are releasing soon. I did have some performance issues on an older machine. But I think it was less to do with destroying and rebuilding and more so to do with having a lot of editors on the same page.

#26 @azaozz
12 months ago

  • Resolution set to maybelater
  • Status changed from reopened to closed

In Fieldmanager, we reinit the editors after the user drags them.

Yeah, that should look better. Only concern is possible "choppy" dragging of a big/complex element that contains the editor iframe.

Looking at https://github.com/alleyinteractive/wordpress-fieldmanager/blob/master/js/richtext.js#L58-L95:

  • You can check if the Text editor is currently being used by doing ed.isHidden().
  • If true, can re-initialize the instance without restoring the content, as the latest content is in the textarea and MCE will load it on initialization.
  • If false (i.e. the Visual editor is being used) will have to "save" to the textarea first.

So the code will be something like (untested):

if ( ! ed.isHidden() ) {
  ed.save();
}

tinymce.remove( ed );
tinymce.init( tinyMCEPreInit.mceInit[ this.id ] );

This should work in WP 4.3+ as the textarea value is run through the JS wpautop. There is no need to set the content or switch editors. For older versions you'll still need to load the content in the new instance when MCE is being used to keep the <p> tags.

I'm not sure what can be added to wp_editor() to make this simpler/easier. It is quite straightforward to implement in a plugin. Going to re-close as "maybelater". Feel free to reopen and attach a patch if you come up with something that can improve it.

#27 @ocean90
5 months ago

#36321 was marked as a duplicate.

Note: See TracTickets for help on using tickets.