WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 3 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 (21)

comment:1 ocean902 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')

comment:2 jakemgold2 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.

comment:3 nacin2 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.

comment:4 azaozz2 years ago

The problem is that TinyMCE, once initialized cannot be moved in the DOM, or rather the browsers cannot handle it being moved. That's why the errors are so inconsistent in different browsers. 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.

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

comment:5 follow-up: azaozz2 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.

comment:6 follow-up: goto102 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.

comment:7 in reply to: ↑ 6 azaozz2 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 2 years ago by azaozz (previous) (diff)

comment:8 goto102 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.

comment:9 azaozz2 years ago

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

Per IRC's 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.

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

comment:10 follow-up: nacin2 years ago

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

comment:11 in reply to: ↑ 10 azaozz2 years ago

Replying to nacin:

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

comment:12 follow-up: goto102 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!

comment:13 in reply to: ↑ 12 ; follow-up: azaozz2 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.

comment:14 in reply to: ↑ 13 ; follow-up: JPry2 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.

comment:15 in reply to: ↑ 14 azaozz2 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 2 years ago by azaozz (previous) (diff)

comment:16 simonwheatley22 months ago

  • Cc simon@… added

comment:17 in reply to: ↑ 5 ; follow-up: devesine16 months 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);
});

comment:18 deanmarktaylor16 months ago

  • Cc deanmarktaylor added

comment:19 sscovil14 months ago

  • Cc sscovil@… added

comment:20 in reply to: ↑ 17 bjorsq10 months 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);
});

comment:21 ircbot3 months ago

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

Note: See TracTickets for help on using tickets.