Ticket #3826 (closed defect (bug): fixed)

Opened 5 years ago

Last modified 4 years ago

TinyMCE misconfiguration causes erronous tag replacements

Reported by: novasource Owned by: 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 Download (1.0 KB) - added by rob1n 5 years ago.
allow-div.diff Download (630 bytes) - added by andy 5 years ago.

Change History

  • 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

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

  • 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

  • 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

  • Owner changed from anonymous to foolswisdom
  • Owner changed from foolswisdom to andy
  • Milestone changed from 2.1.3 to 2.2

rob1n5 years ago

comment:8   andy5 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. ;-)

  • Milestone changed from 2.2 to 2.3

Likely relates to #3617 .

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.

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

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.

andy5 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

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

Indeed very annoying.

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

  • Keywords early added
  • Status changed from new to closed
  • Resolution set to fixed

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

comment:19 in reply to: ↑ description ; follow-up: ↓ 20   bentrem4 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

comment:20 in reply to: ↑ 19   novasource4 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.