Duplicating Embed-Codes

[Bichareh]

Weird issue in WP 3.5 RC4...

If I paste any embed-code in html-mode, change into visual mode and back to html, it duplicates the code.

e.g. this code...
Before (1301 character):

<object id="flashObj" width="630" height="534" classid="clsid:D27CDB6E-AE6D-11cf-96B8-444553540000" codebase="http://download.macromedia.com/pub/shockwave/cabs/flash/swflash.cab#version=9,0,47,0"><param name="movie" value="http://c.brightcove.com/services/viewer/federated_f9?isVid=1" /><param name="bgcolor" value="#FFFFFF" /><param name="flashVars" value="videoId=1999115218001&playerID=18295496001&playerKey=AQ~~,AAAAABumiUQ~,d-Jwax1bUNkyqTy8Pr8lycLwfYHEbxsR&domain=embed&dynamicStreaming=true" /><param name="base" value="http://admin.brightcove.com" /><param name="seamlesstabbing" value="false" /><param name="allowFullScreen" value="true" /><param name="swLiveConnect" value="true" /><param name="allowScriptAccess" value="always" /><embed src="http://c.brightcove.com/services/viewer/federated_f9?isVid=1" bgcolor="#FFFFFF" flashVars="videoId=1999115218001&playerID=18295496001&playerKey=AQ~~,AAAAABumiUQ~,d-Jwax1bUNkyqTy8Pr8lycLwfYHEbxsR&domain=embed&dynamicStreaming=true" base="http://admin.brightcove.com" name="flashObj" width="630" height="534" seamlesstabbing="false" type="application/x-shockwave-flash" allowFullScreen="true" swLiveConnect="true" allowScriptAccess="always" pluginspage="http://www.macromedia.com/shockwave/download/index.cgi?P1_Prod_Version=ShockwaveFlash"/></object>

After (2153 character):

<object width="630" height="534" classid="clsid:D27CDB6E-AE6D-11cf-96B8-444553540000" codebase="http://download.macromedia.com/pub/shockwave/cabs/flash/swflash.cab#version=9,0,47,0"><param name="movie" value="http://c.brightcove.com/services/viewer/federated_f9?isVid=1" /><param name="bgcolor" value="#FFFFFF" /><param name="flashVars" value="videoId=1999115218001&amp;playerID=18295496001&amp;playerKey=AQ~~,AAAAABumiUQ~,d-Jwax1bUNkyqTy8Pr8lycLwfYHEbxsR&amp;domain=embed&amp;dynamicStreaming=true" /><param name="base" value="http://admin.brightcove.com" /><param name="seamlesstabbing" value="false" /><param name="allowFullScreen" value="true" /><param name="swLiveConnect" value="true" /><param name="allowScriptAccess" value="always" /></object><object width="630" height="534" classid="clsid:d27cdb6e-ae6d-11cf-96b8-444553540000" codebase="http://download.macromedia.com/pub/shockwave/cabs/flash/swflash.cab#version=6,0,40,0" bgcolor="#FFFFFF"><param name="src" value="http://c.brightcove.com/services/viewer/federated_f9?isVid=1" /><param name="flashvars" value="videoId=1999115218001&amp;playerID=18295496001&amp;playerKey=AQ~~,AAAAABumiUQ~,d-Jwax1bUNkyqTy8Pr8lycLwfYHEbxsR&amp;domain=embed&amp;dynamicStreaming=true" /><param name="base" value="http://admin.brightcove.com" /><param name="seamlesstabbing" value="false" /><param name="allowfullscreen" value="true" /><param name="swliveconnect" value="true" /><param name="allowscriptaccess" value="always" /><param name="pluginspage" value="http://www.macromedia.com/shockwave/download/index.cgi?P1_Prod_Version=ShockwaveFlash" /><embed width="630" height="534" type="application/x-shockwave-flash" src="http://c.brightcove.com/services/viewer/federated_f9?isVid=1" flashvars="videoId=1999115218001&amp;playerID=18295496001&amp;playerKey=AQ~~,AAAAABumiUQ~,d-Jwax1bUNkyqTy8Pr8lycLwfYHEbxsR&amp;domain=embed&amp;dynamicStreaming=true" base="http://admin.brightcove.com" seamlesstabbing="false" allowfullscreen="true" swliveconnect="true" allowscriptaccess="always" pluginspage="http://www.macromedia.com/shockwave/download/index.cgi?P1_Prod_Version=ShockwaveFlash" bgcolor="#FFFFFF" /></object>

[georgestephanis]

What browser is returning this issue? All?

[Bichareh]

I've tested only in Chrome (23) and Firefox (16). Same results!

[merty]

Tested on IE10, Opera 12.10, Chrome 23, Safari 6 and Firefox 17. Happens on all of them.

[nacin]

So, azaozz has narrowed this down to the change from TinyMCE's "schema" parameter, which we changed in 3.5 to go from HTML4 to HTML5 handling.

You can confirm by going into wp-includes/class-wp-editor.php and removing the 'schema' => 'html5' line at line 348.

I am currently diffing the entire tags and attributes list here: ​https://github.com/tinymce/tinymce/blob/master/jscripts/tiny_mce/classes/html/Schema.js. (It's packed, so not very fun.)

[nacin]

THESE ELEMENTS WERE IN THE HTML4 SCHEMA AND ARE NOT IN HTML5

applet, tt, u, s, strike, big, font, basefont, acronym, dir, center, noframes, isindex

These elements are new to the HTML5 schema

html, head, title, link, meta, section, nav, article, aside, hgroup, header, footer, dialog, mark, progress, meter, time, ruby, rt, rp, figure, figcaption, embed, details, command, source, audio, video, datalist, keygen, output, canvas, mathml, svg, wbr

[azaozz]

Yeah, they are listed as obsolete in ​http://www.w3.org/TR/html5-diff/#obsolete-elements

We have two options: either revert back to html4 and add all the html5 tags, or keep the html5 schema and allow the obsolete/not-supported tags. Seems the second option is better as HTML 5.0 allows more attributes on all tags and generally is a bit more liberal than HTML 4/XHTML 1.

[nacin]

From HTML4 schema to HTML5, a bunch of elements had their attributes removed. I excluded xml:lang and lang from the results — those should be available globally, probably.

script --- language, xml:space

style --- xml:space

object --- declare, codebase, codetype, archive, standby, tabindex, align, border, hspace, vspace

param --- valuetype, type

p --- align

a --- tabindex, onfocus, onblur, charset, name, hreflang, rev, shape, coords

br --- clear

h1 --- align

img --- name, longdesc, align, border, hspace, vspace

h2 --- align

iframe --- longdesc, frameborder, marginwidth, marginheight, scrolling, align

h3 --- align

input --- tabindex, onfocus, onblur, usemap, onselect, onchange, align

select --- tabindex, onfocus, onblur, onchange

textarea --- tabindex, onfocus, onblur, onselect, onchange

label --- onfocus, onblur

button --- tabindex, onfocus, onblur

h4 --- align

h5 --- align

h6 --- align

div --- align

ul --- type, compact

li --- type

ol --- type, compact

dl --- compact

menu --- compact

pre --- width, xml:space

hr --- align, noshade, size, width

legend --- align

table --- summary, width, frame, rules, cellspacing, cellpadding, align, bgcolor

caption --- align

col --- width, align, char, charoff, valign

colgroup --- width, align, char, charoff, valign

thead --- align, char, charoff, valign

tr --- align, char, charoff, valign, bgcolor

th --- abbr, axis, align, char, charoff, valign, nowrap, bgcolor, width, height

form --- onsubmit, onreset, accept

td --- abbr, axis, scope, align, char, charoff, valign, nowrap, bgcolor, width, height

tfoot --- align, char, charoff, valign

tbody --- align, char, charoff, valign

area --- tabindex, onfocus, onblur, nohref

body --- onload, onunload, background, bgcolor, text, link, vlink, alink

[nacin]

From HTML4 to HTML5, a number of elements received *more* attributes. I have excluded these attributes, as they seem to be have been added all over: accesskey, draggable, item, hidden, itemprop, role, spellcheck, subject.

script --- class, dir, style, title, onclick, ondblclick, onmousedown, onmouseup, onmouseover, onmousemove, onmouseout, onkeypress, onkeydown, onkeyup, async

style --- class, style, onclick, ondblclick, onmousedown, onmouseup, onmouseover, onmousemove, onmouseout, onkeypress, onkeydown, onkeyup, scoped

object --- form

param --- class, dir, style, title, onclick, ondblclick, onmousedown, onmouseup, onmouseover, onmousemove, onmouseout, onkeypress, onkeydown, onkeyup

a --- ping, media

br --- dir, onclick, ondblclick, onmousedown, onmouseup, onmouseover, onmousemove, onmouseout, onkeypress, onkeydown, onkeyup

iframe --- dir, onclick, ondblclick, onmousedown, onmouseup, onmouseover, onmousemove, onmouseout, onkeypress, onkeydown, onkeyup, sandbox, seamless

input --- autocomplete, autofocus, form, formaction, formenctype, formmethod, formnovalidate, formtarget, height, list, max, min, multiple, pattern, placeholder, required, step, width, files

select --- autofocus, form

textarea --- autofocus, form, maxlength, placeholder, required, wrap

label --- form

button --- autofocus, form, formaction, formenctype, formmethod, formnovalidate, formtarget

ol --- reversed

menu --- type, label

fieldset --- disabled, form, name

form --- autocomplete, novalidate

area --- media, rel, ping, type

base --- class, dir, style, title, onclick, ondblclick, onmousedown, onmouseup, onmouseover, onmousemove, onmouseout, onkeypress, onkeydown, onkeyup

[WraithKenny]

In case anyone would find looking at the output in a readable way:
​https://gist.github.com/4228771

A summary of the changes from before to after:

1. <object> loses its id attr.
2. <object> is split in two.
3. the first <object> loses its nested <embed> but retains all <param>s correctly.
4. the second <object> inherits a bgcolor attr, which previously existed in only on a <param> and on the <embed>
5. the second <object> has it's codebase attr rewritten to a value not previously present.
6. the <embed> object loses it's name attr.
7. the second's <param>s appear to be rewritten based on the attrs of the <embed>: the "movie" and "bgcolor" <param>s are gone, but there's now a "src" and "pluginspage". It further appears "width", "height", "bgcolor" where simply hoisted past the <param> rewrite to attr on the object.

This looks to me to be some flash back-compatibility rewrite wrapper (version 9 down to 6) in TinyMCE. Perhaps this is activated by the 'schema' => 'html5' flag?

[WraithKenny]

the 5. from above is the evidence for the Compat rewriting: version=9,0,47,0 becomes version=6,0,40,0

[nacin]

Replying to WraithKenny:

This looks to me to be some flash back-compatibility rewrite wrapper (version 9 down to 6) in TinyMCE. Perhaps this is activated by the 'schema' => 'html5' flag?

This summary is awesome, thank you! Can you do this same thing in 3.4.2? TinyMCE does do some rewriting to ensure it works in contenteditable. At this point I am only interested in differences between 3.4.2 and 3.5.

[WraithKenny]

the 3.4.2 comparison (but it isn't a clean install): ​https://gist.github.com/4229053

[WraithKenny]

The rewrite present in 3.4.2:

1. doesn't double the object.
2. doesn't strip the id attr.
3. does strip the "movie" and "bgcolor" params, (same as 3.5)
4. hoists the bgcolor into the object tag (same)
5. converts the version hash on codebase attr (same)
6. "src" param created from embed attr (same)
7. duplicates "flashvars", "swliveconnect", allowScriptAccess" and "pluginspage" by hoisting from embed attr. (sort of the same, 3.5 looks to clean up the dups. better)
8. <embed> has it's name converted to id.
9. embed attrs otherwise left intact but some are doubled. (mostly the same).

[WraithKenny]

if the first object of the 3.5 version could be cleaned out, it'd be much closer.

[azaozz]

22790.patch​ fixes the regression. It still doesn't preserve the <object> exactly but that's how it works in 3.4 and earlier. It's worth it extending/modifying the 'media' plugin for TinyMCE by adding an option there to fully preserve the <object> tags.

[WraithKenny]

22790.patch​ tested and worked.

[WraithKenny]

there are differences still but the double object is gone. the id attr on object gets removed still for example.

[ryan]

In 23112:

Extend allowed elements for object and embed.

Props azaozz
fixes #22790

[nacin]

Re-opening and lowering severity for additional investigation of remaining attributes and children.

[nacin]

For the elements entirely removed from the HTML5 schema, here are their HTML4 schema definitions:

applet[id|class|style|title|codebase|archive|code|object|alt|name|width|height|align|hspace|vspace]
tt[id|class|style|title|lang|xml:lang|dir|onclick|ondblclick|onmousedown|onmouseup|onmouseover|onmousemove|onmouseout|onkeypress|onkeydown|onkeyup]
u[id|class|style|title|lang|xml:lang|dir|onclick|ondblclick|onmousedown|onmouseup|onmouseover|onmousemove|onmouseout|onkeypress|onkeydown|onkeyup]
s[id|class|style|title|lang|xml:lang|dir|onclick|ondblclick|onmousedown|onmouseup|onmouseover|onmousemove|onmouseout|onkeypress|onkeydown|onkeyup]
strike[id|class|style|title|lang|xml:lang|dir|onclick|ondblclick|onmousedown|onmouseup|onmouseover|onmousemove|onmouseout|onkeypress|onkeydown|onkeyup]
big[id|class|style|title|lang|xml:lang|dir|onclick|ondblclick|onmousedown|onmouseup|onmouseover|onmousemove|onmouseout|onkeypress|onkeydown|onkeyup]
font[id|class|style|title|lang|xml:lang|dir|size|color|face]
basefont[id|size|color|face]
acronym[id|class|style|title|lang|xml:lang|dir|onclick|ondblclick|onmousedown|onmouseup|onmouseover|onmousemove|onmouseout|onkeypress|onkeydown|onkeyup]
dir[id|class|style|title|lang|xml:lang|dir|onclick|ondblclick|onmousedown|onmouseup|onmouseover|onmousemove|onmouseout|onkeypress|onkeydown|onkeyup|compact]
center[id|class|style|title|lang|xml:lang|dir|onclick|ondblclick|onmousedown|onmouseup|onmouseover|onmousemove|onmouseout|onkeypress|onkeydown|onkeyup]
noframes[id|class|style|title|lang|xml:lang|dir|onclick|ondblclick|onmousedown|onmouseup|onmouseover|onmousemove|onmouseout|onkeypress|onkeydown|onkeyup]
isindex[id|class|style|title|lang|xml:lang|dir|prompt]

[azaozz]

22790-2.patch​ extends the html5 schema in TinyMCE with all tags and attributes from the html4 schema, more or less merging the two. That we we have good HTML 5.0 compatibility and good back-compat with existing posts.

[nacin]

You can include tinymce-schemas.php​ into a PHP file and you'll end up with two arrays: $html4 and $html5. These both represent the schemas in ​​https://github.com/tinymce/tinymce/blob/master/jscripts/tiny_mce/classes/html/Schema.js. I took that file and executed it, then logged the two function returns as JSON, and there you are.

The format is as follows:

    element => object(
          attributes => object(
               (attribute1) => {}
               (attribute2) => {}
               (attribute3) => {}
               (attribute4) => {}
          ),
          attributeOrder => array( attribute1, attribute2, attribute3, attribute4 ),
          children => object(
               (element) => {}
               (element) => {}
               (element) => {}
          ),
    ),
    element => object( ...

So, if you pluck the attributeOrder key, you'll end up with all valid attributes for that object. This is the primary focus.

You can also look at the children key for elements that are valid immediately inside this element.

[nacin]

tinymce-test.php​ is what I used to generate the comparison comments above.

We can extend this further to:

1. Double-check azaozz's work.
2. Ensure that there are no changes to allowed children.

Hack away. Sorry for the crudeness.

[azaozz]

22790-3.patch is the same as 22790-2.patch but fixes xml:lang so it's not forced but verified.

[nacin]

Three approaches here:

• I manually went into getHTML5 and added elements, attributes, and children. (You can see this in 22790.getHTML5.diff​.)
• koopersmith wrote a script to merge getHTML5 and getHTML4. You can see this in 22790.schema.debug.diff​. Look for things like "WordPress," "sane," "nacin," and "Koop".
• azaozz added everything in the TinyMCE plugin.

Then, Koop used his script to check both my work and azaozz's work. There were a few issues in my work that we resolved 22790.schema.2.diff​ includes those changes).

azaozz's patch is missing lang and xml:lang in a number of places. (Which I don't really care about... should we?)

Additionally, azaozz's patch ignores children. (How could you?!) It only adds elements and attributes. That means that in azaozz's patch, tinymce.activeEditor.schema.isValidChild( 'div', 'big' ) returns false. For me and koop, it returns true.

But, it seems like TinyMCE doesn't care about children validness. (Insert parental joke here?) <div><big>Big</big></div> works as long as <big> is registered with addValidElements, even if it is missing from div's children. Which makes us all wonder if it is a bug, an incomplete API, or just something we're missing.

azaozz is now pinging TinyMCE's Spocke to discuss. Assuming we can just ignore the plight of the children, I am +1 on azaozz's patch, primarily because his results has been backed up by two other methods and a lot of manpower, and secondarily since we've already been doing exactly that all release, for other elements and attributes.

[koopersmith]

An excellent summary. If we can go with azaozz's patch, we should, especially since it's based on what we've been using all cycle. Otherwise, we have the other well-tested overrides waiting in the wings.

[nacin]

In 23119:

Add TinyMCE 3.5.8's Schema.js file in preparation for monkey patching it. see #22790.

[nacin]

In 23120:

Add a sane, inclusive HTML element/attribute schema to TinyMCE.

TinyMCE 3.4.x (shipped with WordPress 3.4.x) had an HTML4-based schema definition, with HTML5 elements added to it. TinyMCE 3.5.x (shipping, again coincidentally, with WordPress 3.5) allows for HTML5 schema support, which also provides for full HTML5 attribute support. The problem is its HTML5 schema excludes all HTML4 elements and attributes that were dropped in the HTML5 spec, which is unacceptable behavior.

This "duck punch" of TinyMCE's Schema.js file creates a new, sane schema. It is TinyMCE's HTML4 and HTML5 schema definitions recursively merged.

Objects are not whitelisted in either schema to allow for embed elements as child nodes, so object, param, and embed remain separately whitelisted in the WordPress TinyMCE plugin. Our attempts to add other attributes in said plugin is now superceded.

props koopersmith, azaozz.
fixes #22790.

[nacin]

After a long work session in IRC, it was determined that addValidElements was insufficient for new elements (acronym, strike, etc.), because it did not set those elements to be allowed children for all other elements. This is a problem because to fix it, we'd need to call addValidChildren for essentially every element. The elements would still work even if they were not allowed; per Spocke they would be treated as custom elements and kind of like spans. But tinymce.activeEditor.schema.isValidChild would return false, and we didn't want to get into the ramifications of that.

At that point, we decided to go with Koop's straightforward duck punch. [23120], and bumpbot in [23121]. The wp-tinymce.js.gz file now includes a minified version of wp-includes/js/tinymce/wp-tinymce-schema.js immediately after core, and before plugins. That will allow our version of Schema.js to replace theirs.

Nice work, everyone.

[azaozz]

Replying to WraithKenny:

...the id attr on object gets removed still for example.

Been looking why this is still happening. The syntax for adding embed as a valid child to object is different from the syntax for adding valid attributes, so we should be doing addValidChildren('+object[embed]') instead of addValidChildren('object[*]').

Here is the description of the ​TinyMCE API function, the ​`valid_children` init setting that gets passed directly to that function, and an ​example of TinyMCE core using it.

Note: this is a fix for 22790.patch/[23112] to make TinyMCE properly recognize <object><embed /></object> tags, it doesn't affect the rest of the schema.

[nacin]

<object></object> without an <embed /> inside now gets stripped entirely.

[azaozz]

Seems to be caused by addValidChildren('object[*]') interfering with the MCE media plugin. 22790-valid-children.patch​ fixes this as well.

[koopersmith]

I've looked through MCE in 3.5 and 3.4 and the various code paths, and this appears to be the correct fix. Allowing all children is too broad, but registering the embed tag without adding it as a child causes large amounts of breakage when the TinyMCE media plugin reads an object.

To quote azaozz from IRC:

It works in 3.4 as the <embed> tag is not mentioned as being allowed child of any other tag, so the check skips it.

The TinyMCE HTML5 schema adds the embed tag, so things begin breaking. This fixes them. +1 for commit.

[nacin]

In 23142:

TinyMCE: Use the correct syntax for allowing an embed element to be a valid child of an object element.

Fixes a bug in TinyMCE when using its HTML5 schema definition.

props azaozz.
see #22790.
for trunk.

[nacin]

In 23144:

TinyMCE: Use the correct syntax for allowing an embed element to be a valid child of an object element.

Fixes a bug in TinyMCE when using its HTML5 schema definition.

props azaozz.
see #22790.
for trunk.
... with the right patch this time. [23142] was done in error.

[nacin]

In 23146:

TinyMCE: Use the correct syntax for allowing an embed element to be a valid child of an object element.

Fixes a bug in TinyMCE when using its HTML5 schema definition.

props azaozz.
fixes #22790.
for the 3.5 branch.

[nacin]

In 23149:

Delete the patch of TinyMCE's schema; new attempt. see #22790. see #22842. for the 3.5 branch.

[nacin]

In 23150:

Delete the patch of TinyMCE's schema; new attempt. see #22790. see #22842. actually for the 3.5 branch.

[nacin]

In 23151:

Add TinyMCE's Schema.js to prepare for patching it. see #22790. see #22842. for trunk.

[nacin]

In 23152:

Add TinyMCE's Schema.js to prepare for patching it. see #22790. see #22842. for the 3.5 branch.

[nacin]

In 23153:

Restore the duck punch of TinyMCE's schema from [23120], along with updated rules for all HTML5 elements, as painstakingly audited against the HTML5 spec.

Remove conflicting and unnecessary code in the WP TinyMCE plugin.

Fixes all known regressions when working with the embed, object, video, audio, source, and param elements in TinyMCE.

props markjaquith, koopersmith.
fixes #22790, #22842.
fixes #22842.

[nacin]

In 23154:

Restore the duck punch of TinyMCE's schema from [23120], along with updated rules for all HTML5 elements, as painstakingly audited against the HTML5 spec.

Remove conflicting and unnecessary code in the WP TinyMCE plugin.

Fixes all known regressions when working with the embed, object, video, audio, source, and param elements in TinyMCE.

props markjaquith, koopersmith.
fixes #22790, #22842.

Merges [23153] to the 3.5 branch.