WordPress.org

Make WordPress Core

Opened 13 years ago

Closed 12 years ago

Last modified 12 years ago

#3826 closed defect (bug) (fixed)

TinyMCE misconfiguration causes erronous tag replacements

Reported by: novasource Owned by: andy
Milestone: 2.5 Priority: high
Severity: normal Version: 2.2
Component: TinyMCE Keywords: has-patch early
Focuses: Cc:
PR Number:

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 (2)

3826.diff (1.0 KB) - added by rob1n 13 years ago.
allow-div.diff (630 bytes) - added by andy 12 years ago.

Download all attachments as: .zip

Change History (22)

#1 @foolswisdom
13 years ago

  • Milestone changed from 2.1.1 to 2.2
  • Summary changed from Editor messes up tags to Editor replaces div with p including the div's class

Remember to give good Summary's

#2 @Otto42
13 years ago

  • 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.

#3 @linusmartensson
13 years ago

  • 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

#4 @linusmartensson
13 years ago

  • Milestone changed from 2.2 to 2.1.3
  • 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

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

#5 @foolswisdom
13 years ago

  • Owner changed from anonymous to foolswisdom

#6 @foolswisdom
13 years ago

  • Owner changed from foolswisdom to andy

#7 @foolswisdom
13 years ago

  • Milestone changed from 2.1.3 to 2.2

@rob1n
13 years ago

#8 @andy
13 years ago

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. ;-)

#9 @foolswisdom
13 years ago

  • Milestone changed from 2.2 to 2.3

Likely relates to #3617 .

#10 @Linusmartensson
13 years ago

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.

#11 @miklb
12 years ago

  • 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.

#12 @andy
12 years ago

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.

@andy
12 years ago

#13 @miklb
12 years ago

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

#14 @torbens
12 years ago

  • Keywords has-patch added

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

#15 @f00f
12 years ago

Indeed very annoying.

#16 @markjaquith
12 years ago

  • Milestone changed from 2.3 to 2.4 (next)
  • Priority changed from normal to high

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

#17 @foolswisdom
12 years ago

  • Keywords early added

#18 @ryan
12 years ago

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

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

#19 in reply to: ↑ description ; follow-up: @bentrem
12 years ago

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

#20 in reply to: ↑ 19 @novasource
12 years ago

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.

Note: See TracTickets for help on using tickets.