WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 6 years ago

Last modified 6 years ago

#23037 closed defect (bug) (fixed)

Want to use twitter-bootstrap icons, but TinyMCE strips them out

Reported by: ruud@… Owned by:
Milestone: 3.9 Priority: high
Severity: blocker Version: 3.5
Component: TinyMCE Keywords: dev-feedback
Focuses: Cc:

Description

The 3.5 tinyMCE editor strips out empty HTML tags.
In this way I cannot use bootstrap-icon as per instructions from the twitter-bootstrap page:
http://twitter.github.com/bootstrap/base-css.html#icons

It works correctly when I use a &nbsp; or regular space within the <i> tags.

Might there be a way in which TinyMCE does not strip out empty html elements, when a class or classes is set on this element?

Change History (14)

#1 @ruud@…
8 years ago

  • Type changed from feature request to defect (bug)

Changed to defect (bug), because it might as well be.
In the 3.4.2 editor the <i> tags are converted to <em> elements, but aren't stripped out when empty. So I guess when other users open up there content into the 3.5 editor some content styling add-ons via empty elements will get stripped out.

#2 @SergeyBiryukov
8 years ago

  • Component changed from Editor to TinyMCE

Related: #22477

#3 @azaozz
8 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Closing as duplicate of #22477 as this is the same problem/affected by the same code in TinyMCE.

#4 @azaozz
7 years ago

In 27083:

TinyMCE: don't replace <i> with <em> and <b> with <strong> and don't remove them when empty, see #24067, see #23037.

#5 @nacin
7 years ago

  • Milestone set to 3.9
  • Priority changed from normal to high
  • Resolution duplicate deleted
  • Severity changed from normal to blocker
  • Status changed from closed to reopened

Re-opening and setting this to 3.9. #22477 was closed more than a year ago.

I'm setting this ticket to blocker. In 3.5, we had a last-minute scramble to fix all of the issues in TinyMCE's restrictive HTML5 schema. See #22790. That is not happening for 3.9.

I don't know what happened here but I think we need to review to make sure that [26876] didn't cause any other regressions. Our 3.5 schema was extremely forgiving and I have trouble believing this is the only regression.

Last edited 6 years ago by SergeyBiryukov (previous) (diff)

#6 @TobiasBg
7 years ago

Related: #26986 for problems with empty <span> tags, which was to some degree discussed (and excluded from a permanent solution) in #22477

#7 @azaozz
7 years ago

Our 3.5 schema was extremely forgiving and I have trouble believing this is the only regression.

This is not a regression. It was originally patched after we patched the schema. It is specifically to not replace <i> with <em> and <b> with <strong> which has been the default behavior since TinyMCE 2.0 was released back in 2006. I believe that was done to promote the use of Semantic HTML, the same reason the Text editor inserts <em> when the i button is clicked.

A good way to ensure HTML 4 compatibility is to make unit tests for it, see #27014.

#8 @azaozz
6 years ago

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

Tests for support for obsolete HTML 5.0 elements and attributes in TinyMCE: https://core.trac.wordpress.org/browser/trunk/tests/qunit/editor/tinymce/html/Obsolete.js

#9 @ruud@…
6 years ago

Knowing little of the unit tests in WordPress (and unit testing in general) I was curious how the linked file looked like.
Having gone over the file, it struck me that it seems there is no test for the case were <i></i> should not be converted to <em></em> or stripped out completely. Probably these are covered in some other unit test? If not, could that be added to the test?

#10 @azaozz
6 years ago

Replacing <i> with <em> is a setting in TinyMCE, on by default. The same can be done for any html tags. Also any tag or attribute can be set to "invalid" and will be removed on cleanup.

In the unit tests we are testing the handling of tags and attributes that are deprecated in html5 and using the default settings in TinyMCE. There are tests for changing the default settings, not exactly for the <i>/<em> and <b>/<strong> replacements.

#11 @ruud@…
6 years ago

I can confirm that <i class="icon icon-glass"></i> tags are not converted to <em> or stripped out in the current trunk version, great!
As I understand correctly from you answer the (non-default) behavior of <i> not being converted over to <em> (and <b> to <strong>) is not tested for?
Nor is the non-default behavior of not removing empty <i></i> tags?

I favor tests for these non-default situations as well, because if this behavior gets changed by accident in future releases this will affect users who use icon fonts and the like.

As said I'm not familiar with Unit tests that much, but I guess this is something which could work?

text = 'Not transform <i> into <em>';
testString = '<i class="icon icon-glass">Not empty</i>';
editor.setContent( testString );
equal( editor.getContent(), testString, text );

text = 'Not strip out empty <i> tag';
testString = '<i class="icon icon-glass"></i>';
editor.setContent( testString );
equal( editor.getContent(), testString, text );

text = 'Not transform <b> into <strong>';
testString = '<b>Bolded text</b>';
editor.setContent( testString );
equal( editor.getContent(), testString, text );

text = 'Not strip out empty <b> tag';
testString = '<b class="test"></b>';
editor.setContent( testString );
equal( editor.getContent(), testString, text );

#12 @ruud@…
6 years ago

  • Keywords dev-feedback added

#13 @DonSailieri
6 years ago

Still relevant with 3.9.1, +1 for a fix!

Note: See TracTickets for help on using tickets.