Ticket #3826 (closed defect: fixed)

Opened 1 year ago

Last modified 6 months ago

TinyMCE misconfiguration causes erronous tag replacements

Reported by: novasource Assigned to: andy
Priority: high Milestone: 2.5
Component: TinyMCE Version: 2.2
Severity: normal Keywords: has-patch early
Cc:

Description

If this code is stored in the database:

<li>
<div class="g2image_float_right"><wpg2id>6355</wpg2id></div>
<p><strong>some boldfaced text</strong> Other text.</li>

the editor converts it to this when you open it in an editor:

	<li>
<p class="g2image_float_right"><wpg2id></wpg2id>6355

<strong>some boldfaced text</strong> Other text.</li>

The editor stripped the div tags, put the div tags' content inside the p tags, and assigned the div's class to the p.

Attachments

3826.diff (1.0 kB) - added by rob1n on 04/11/07 17:12:55.
allow-div.diff (0.6 kB) - added by andy on 06/13/07 17:20:34.

Change History

02/20/07 22:24:26 changed by foolswisdom

  • summary changed from Editor messes up tags to Editor replaces div with p including the div's class.
  • milestone changed from 2.1.1 to 2.2.

Remember to give good Summary's

02/21/07 22:07:03 changed by Otto42

  • component changed from General to TinyMCE.

This is a TinyMCE issue of some sort, I've seen a lot of complaints about div's becoming p's on the support forums. They all end up being visual editor related.

03/04/07 23:33:09 changed by linusmartensson

  • summary changed from Editor replaces div with p including the div's class to Editor replaces div with p including the div's class, several other tags are also replaced in a similar fashion.

There's more problems related to this. I used <embed> to include a flash from youtube on a page in the text editor whilst testing things. The <embed> was replaced with <ibed>, <em> is a tag somewhat equivalent to <i>, but less use. In any case the replacement is outright stupid. I would strongly consider finding a replacement for TinyMCE until they work out dumb bugs like this.

Linus

03/04/07 23:55:04 changed by linusmartensson

  • summary changed from Editor replaces div with p including the div's class, several other tags are also replaced in a similar fashion to TinyMCE misconfiguration causes erronous tag replacements.
  • milestone changed from 2.2 to 2.1.3.

On second thought, I looked into this a bit over at http://wiki.moxiecode.com/index.php/TinyMCE:Configuration/valid_elements

Turns out this is just a TinyMCE malconfiguration.

Here's the current code:

// Set up init variables
$valid_elements = 'p/-div[*],-strong/-b[*],-em/-i[*],-font[*],-ul[*],-ol[*],-li[*],*[*]';
$valid_elements = apply_filters('mce_valid_elements', $valid_elements);

I'd recommend rewriting it to use negation rather than approval, seeing as *[*] is added at the end, which approves all elements anyhow. Not to mention the text editor would allow illegal statements in any case as long as the visual editor is disabled.

tiny_mce_config.php:

$valid_elements = '*[*]';
$valid_elements = apply_filters('mce_valid_elements', $valid_elements);
$invalid elements = apply_filters('mce_invalid_elements', '');

Then you'd add this line to the init section:

invalid_elements : "<?php echo $invalid_elements; ?>",

Now you could just add basic filters to set up the valid / invalid elements without strange configurations confusing users.

Linus

03/05/07 17:09:46 changed by foolswisdom

  • owner changed from anonymous to foolswisdom.

03/05/07 17:10:28 changed by foolswisdom

  • owner changed from foolswisdom to andy.

03/28/07 00:43:28 changed by foolswisdom

  • milestone changed from 2.1.3 to 2.2.

04/11/07 17:12:55 changed by rob1n

  • attachment 3826.diff added.

04/12/07 02:15:47 changed by andy

The div/p conversion was added to deal with a problem with themes I think. Switch it back to *[*] if you like but then you become the endpoint for all complaints on this. ;-)

04/12/07 18:27:16 changed by foolswisdom

  • milestone changed from 2.2 to 2.3.

Likely relates to #3617 .

04/12/07 23:09:55 changed by Linusmartensson

Andy: In those cases I'd contribute the problem to faulty theme coding. :p

Since either way seemingly has it's own unique problems there should at least be an option under Writing to toggle code filtering, I'd say.

06/08/07 00:20:41 changed by miklb

  • version changed from 2.1 to 2.2.

It should not be WordPress's job to fix themes on the fly. The editor should output what is being put into it. This is really a bad solution, if it is indeed the case.

I really would like to see this fixed.

06/13/07 17:11:57 changed by andy

I don't want to be the only person who knows which line of code causes this behavior. tiny_mce_config.php sets and filters $valid_elements. The syntax for this tinymce parameter is explained in the tinymce wiki.

p/-div[*] means "p is allowed; div will be converted to p and empty divs are not allowed; all attributes are allowed on this."

So if you want to remove the div-to-p conversion, remove that entire term from the valid_elements string.

06/13/07 17:20:34 changed by andy

  • attachment allow-div.diff added.

06/14/07 14:52:52 changed by miklb

thanks for sharing that andy, it does indeed resolve the div->p issue. Now if it were only that easy to fix the tables issues. http://trac.wordpress.org/ticket/2992

09/11/07 11:44:01 changed by torbens

  • keywords set to has-patch.

I think this is a non-critical patch that should find its way into v2.3. The issue can be very annoying...

09/12/07 13:42:53 changed by f00f

Indeed very annoying.

09/12/07 21:48:13 changed by markjaquith

  • priority changed from normal to high.
  • milestone changed from 2.3 to 2.4 (next).

Too close to release -- we'll get this into 2.4 early in the development cycle.

09/13/07 03:58:28 changed by foolswisdom

  • keywords changed from has-patch to has-patch early.

12/18/07 20:46:10 changed by ryan

  • status changed from new to closed.
  • resolution set to fixed.

(In [6405]) Fix tinymce valid elements configuration. Props linusmartensson. fixes #3826

(in reply to: ↑ description ; follow-up: ↓ 20 ) 12/25/07 04:33:37 changed by bentrem

Replying to novasource:

If this code is stored in the database: {{{ <li> <div snipped . the editor converts it to this when you open it in an editor: {{{ <li> <p snipped .

For the sake of completeness: is there any good reason for inserting whitespace in front of the <li>? IMNSHO it just adds to the eye-noise, when trying to keep track of what TinyMCE has changed/polluted/adjusted.

p.s. greets

(in reply to: ↑ 19 ) 01/09/08 01:17:12 changed by novasource

For the sake of completeness: is there any good reason for inserting whitespace in front of the <li>? IMNSHO it just adds to the eye-noise, when trying to keep track of what TinyMCE has changed/polluted/adjusted.

I think whitespace should be used to hierarchically indent tags to make them more readable. Other than that, I think whitespace should be avoided.