Opened 5 months ago

Closed 5 months ago

Last modified 5 months ago

#22842 closed defect (bug) (fixed)

Object Within Video?

Reported by: miqrogroove Owned by: nacin
Priority: highest omg bbq Milestone: 3.5
Component: TinyMCE Version: 3.5
Severity: blocker Keywords: commit
Cc:

Description

Following up to #22790,

By looking in the TinyMCE Media plugin, I just wanted to wonder out loud if there is a concern about elements other than embed.

From the plugin source we have:

ed.schema.addValidElements('object[id|style|width|height|classid|codebase|*],param[name|value],embed[id|style|width|height|type|src|*],video[*],audio[*],source[*]');

So are the HTML 5 audio, video, source, etc. elements working? I did a quick comparison to the included schema file then tested what looked like a potential problem area.

For example, code found at http://www.w3schools.com/html/html_videos.asp

<video width="320" height="240" controls="controls">
  <source src="movie.mp4" type="video/mp4">
  <source src="movie.ogg" type="video/ogg">
  <source src="movie.webm" type="video/webm">
  <object data="movie.mp4" width="320" height="240">
    <embed src="movie.swf" width="320" height="240">
  </object> 
</video>

When pasted into the Text tab in 3.5 and switched to Visual and back, the code looks quite different afterward.

Does object need to be a valid child of video? And so on?

Attachments (8)

22842.diff (1.6 KB) - added by nacin 5 months ago.
Toss in the towel.
22842.patch (892 bytes) - added by azaozz 5 months ago.
22842.3.diff (2.1 KB) - added by koopersmith 5 months ago.
22842.4.diff (2.8 KB) - added by koopersmith 5 months ago.
22842.5.diff (4.7 KB) - added by koopersmith 5 months ago.
22842.6.diff (8.1 KB) - added by koopersmith 5 months ago.
22842.7.diff (10.7 KB) - added by koopersmith 5 months ago.
22842.8.diff (11.4 KB) - added by koopersmith 5 months ago.

Download all attachments as: .zip

Change History (35)

  • Milestone changed from Awaiting Review to 3.5
  • Priority changed from normal to highest omg bbq
  • Severity changed from normal to blocker

This causes a JS error and is a regression.

Also: If the whitespace is removed from between the elements, the behavior on subsequent tab switches becomes amusing.

nacin5 months ago

Toss in the towel.

azaozz5 months ago

Both @nacin and I were patching this simultaneously and finished at precisely the same time :)

At this point I'm leaning towards 22842.diff​ (revert back to html4 schema). 22842.patch works for video and audio but wondering how many more errors are there in the html5 schema...

attachment:22842.3.diff updates the underlying HTML5 schema to support flow-based elements where approprate (i.e. video, audio, and object) — note that embed is included in that group. It also adds support for the html5 track element. This makes our additional adjustments in the WP tinymce plugin redundant, so those have been removed.

22842.3.diff works well here. Tested with all problematic/recently reported tags and attributes.

attachment:22842.4.diff is the result of auditing the HTML5 spec for every attribute listed in attachment:22842.diff, to ensure that the resulting schema will include everything possible with that patch.

We've added:

  • Appropriate child elements to canvas.
  • Additional attributes to audio and video.
  • The summary element.
  • command elements can contain other command elements.

attachment:22842.5.diff adds support for the bdi and data elements.

.5 works well for me

comment:9 follow-up: ↓ 10   miqrogroove5 months ago

22842.5.diff is a huge improvement.

I may have broken it here though. Try pasting into Text tab:

<details>blah <summary><span>hello</span> blah</summary> </details>

Then switch tabs repeatedly.

comment:10 in reply to: ↑ 9   koopersmith5 months ago

Replying to miqrogroove:

22842.5.diff is a huge improvement.

I may have broken it here though. Try pasting into Text tab:

<details>blah <summary><span>hello</span> blah</summary> </details>

Then switch tabs repeatedly.

Given that we're just introducing support for the summary element and that TinyMCE itself supports an extremely limited (and quite janky) interaction with the details and summary elements, we're going to consider "support" as meaning "doesn't delete the node". The non-breaking spaces that pile up can be addressed in a future WP or TinyMCE release.

Test case posted above behaves the same in 3.4.2. Put it on the ignore list.

Appears to be deleting code here:

<details>Hello world.<summary><data value="93" /></summary></details>

Sorry, ignore that one too. I think I had my 3.4.2 window still open.

Here we go:

<table><tr><td> <data value="93">19+74 hexes (93 total)</data></td></tr></table>

That works:

<table><tr><td><span><data value="93">19+74 hexes (93 total)</data></span></td></tr></table>

+1 commit 22842.5.diff​

This thing is ready.

22842.6.diff is even better than 22842.5.diff, thinking we are covering a lot of tags and attributes that aren't even supported in the browsers yet or are just being added.

  • Keywords commit added

+1 to commit

+1 to azaozz's +1

attachment:22842.7.diff removes and accounts for the extra (now redundant) HTML4-compat code that was committed on #22790.

attachment:22842.8.diff audits global attributes, phrasing content, and flow content groups, adding back any elements that the spec still defines. Corrects mathml to math, adds back s and u, and adds numerous missing attributes.

In 23149:

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

In 23150:

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

In 23151:

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

In 23152:

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

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

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.

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.

Impressive!

Best wishes on the 3.5 release.

Happy Holidays!

Note: See TracTickets for help on using tickets.