Make WordPress Core

Opened 10 years ago

Closed 5 years ago

#28940 closed defect (bug) (worksforme)

TinyMCE strips out empty <i> tags, breaking font awesome

Reported by: donsailieri's profile DonSailieri Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.9.1
Component: TinyMCE Keywords: has-patch needs-testing dev-feedback
Focuses: Cc:

Description (last modified by SergeyBiryukov)

See also #22477 and #23037.

Still relevant on 3.9.1, clean install on newest Chrome.

Attachments (1)

28940.patch (1.2 KB) - added by azaozz 10 years ago.

Download all attachments as: .zip

Change History (20)

#1 @SergeyBiryukov
10 years ago

  • Description modified (diff)
  • Milestone changed from Awaiting Review to 4.0

Confirmed with Chrome 35 in both 3.9 and current trunk.

@azaozz
10 years ago

#2 @azaozz
10 years ago

The empty <i> are only removed when there is no other content in the parent block (<p><i></i> test</p> stays intact). The only way around that is to pad the empty <i> on 'BeforeSetContent' and remove the padding on 'SaveContent'.

Please test 28940.patch asap so it can make it in 4.0 :)

This ticket was mentioned in IRC in #wordpress-dev by DrewAPicture. View the logs.


10 years ago

#4 follow-up: @DrewAPicture
10 years ago

  • Keywords dev-feedback has-patch added

I have to be honest, I don't really understand what precisely I should be testing here with the patch. @azaozz: Can you possibly provide an example of what markup should be used to test this?

#5 in reply to: ↑ 4 @azaozz
10 years ago

  • Keywords needs-testing added; dev-feedback removed
  • Milestone changed from 4.0 to Future Release

Replying to DrewAPicture:

Was hoping the original reporter or somebody else affected by this would test the patch.

Seems there's no much interest in fixing this border case. That patch can also be added in a small TinyMCE plugin, or a bigger plugin that facilitates using these icons inside the editor.

#6 @ruud@…
10 years ago

  • Keywords dev-feedback added

Hi Azaozz,

If any-one wants to test this patch, just try to enter any font-awesome characters. You can see how by looking up the 'How to use' paragraph at this link:
http://getbootstrap.com/2.3.2/base-css.html#icons

I tested the patch and can confirm that without the patch the empty <i> tags are removed (version WordPress 4.0-beta3-20140809).
After applying the patch empty <i> tags are unchanged when you switch between the visual and text editors.

I want to make a case for this patch to be included in version 4.0, because I think it is important to have a consistent behavior. In older versions the empty <i> tags are not stripped out so people can use it to enter font-awesome icons / bootstrap 2 Glyphicons. If version 4.0 would break that, they can lose these characters when re-opening older posts, switching from text to visual and saving the post. This might even happen without them noticing it, but the meaning of their content could have changed much because of it.

However: I noticed that Bootstrap 3 now uses an empty span tag to enter Glypicons (http://getbootstrap.com/components/) and I'm sorry to say but these also get parsed out when switching between the visual and text editors :(

Will make a new bug report for this.

#7 follow-up: @ruud@…
10 years ago

??

When investigating the removal of the empty <span> tags, I noticed that the empty <i> tags are not being removed even after removing the 28940.patch.

I checked in the code, the patch lines are not there, added a define('SCRIPT_DEBUG', true); line to force the use of the regular .js files (sorry forgot that one before) and the empty <i> tags do not get removed when switching from visual to text and back.

Am I going nuts?? or is this fixed via another way and already included in the current beta3?

(using WordPress 4.0-beta3-20140809)

Last edited 10 years ago by ruud@… (previous) (diff)

#8 in reply to: ↑ 7 @azaozz
10 years ago

Replying to ruud@…:

I noticed that the empty <i> tags are not being removed even after removing the 28940.patch.

Yes, currently empty <i> tags are only removed when there is no other content in the parent block tag, see comment:2.

Empty spans are always removed though as WebKit/Blink have the "bad habit" of inserting tons of them in many different cases, and this behavior changes from version to version.

Still thinking this should be a plugin material. Empty inline tags that are needed for styling can be properly filtered and padded, the external stylesheet can be added, some sort of "Select an icon" dialog would be pretty nice, etc.

Last edited 10 years ago by azaozz (previous) (diff)

#9 @ruud@…
10 years ago

See #29171

Hi Azaozz,

I agree with you on the removing of empty spans. However your patch (and the 29171.patch) seems to pad only those tags with an extra attribute and is not padding real empty span, which get removed subsequently in the process I presume?

I disagree somewhat with you about whether or not this should be plugin material. Yes new features (like supporting schema) could very well be plugin material, but these simple use cases (with empty i and span tags) were working before in older WP versions, so stripping these out and providing plugins to compensate for that, is still a loss of functionality from an end-user perspective.

Is it an idea to combine both patches and use these in version 4.0?

#10 @rcain
9 years ago

i would also like to support calls for a fix to this issue please.

my particular case (and i suspect it is not uncommon), is when creating a table containing (only) font-awesome ticks as i tags. (obviously parent td's have no other content).

PS. i tried the patch #28940 above, but to no avail in my example - it just strips all my hard entered table data (ticks)!

i am using latest WP 4.2.4.

(currently investigating whether there are any alternative HTML/CSS workarounds).


#11 @rcain
9 years ago

PS. adding to my comment above: as a workaround, i am using plugin shortcodes (in my case [fa class="fa fa-check"] or [icon name=icon-check] ) - which gets around the problem.

therefore supposing the easiest general workaround for people is to define a similar shortcode for empty 'i' tags in their theme functions.php (if there isn't already one defined by plugins) - or else to produce a small standalone plugin to do similar.

#12 @lisota
9 years ago

A fix for this is needed. icon fonts routinely use empty elements with a class defined. Font Awesome uses the <i> tag. Icomoon likes to use <span>.

I don't buy that this is plugin material. This is TinyMCE being too opinionated and aggressive on what it decides to remove from HTML. These are common use cases with the rise in use of icon fonts.

#13 follow-ups: @NowComGreg
9 years ago

I just came across this exact issue.

As @lisota said, a fix for this is needed.

A work around can be found here for now: http://stv.whtly.com/2014/07/19/prevent-wordpress-tinymce-editor-removing-font-awesome-tags/

Just add a comment tag inbetween the <i> like so

<i class="fa fa-gamepad"><!-- icon --></i>

#14 @xavierserranoa
8 years ago

not sure if I'm allow to post here or this but on the mention by @azaozz of maybe creating a plugin or something i have in the past week working on something myself just need to sort out a few things but it i have it on a very workable and usable state. i just need to make <i> tags with :before content icons behave as if they where a regular letter/character.

http://gph.is/2bTBWtF

#15 @dgwyer
8 years ago

Was just about to open a new ticket when I came across this one.

I'm also seeing Font Awesome tags being stripped out when switching between Text > Visual > Text editor modes.

It's such a common task to include font icons in markup these days that this should be reconsidered for inclusion in core rather than a plugin.

Perhaps we could maintain a white list of exceptions for empty tags? e.g. For Font Awesome this could be empty <i> tags containing an 'fa' class. Or, should this be more generic and just cover tag names only. e.g. All empty <i> tags?

#16 in reply to: ↑ 13 @jan.mazanek
7 years ago

Fix is needed. Thank NowComGreg for temporary solution:

Just add a comment tag inbetween the <i> like so

<i class="fa fa-gamepad"><!-- icon --></i>

#17 @marcmaurer
6 years ago

Problem is still there. Any updates to this issue?

#18 in reply to: ↑ 13 @Garavani
6 years ago

Replying to NowComGreg:

I just came across this exact issue.

As @lisota said, a fix for this is needed.

A work around can be found here for now: http://stv.whtly.com/2014/07/19/prevent-wordpress-tinymce-editor-removing-font-awesome-tags/

Just add a comment tag inbetween the <i> like so

<i class="fa fa-gamepad"><!-- icon --></i>

You are the man!
Many thanks!

#19 @azaozz
5 years ago

  • Milestone Future Release deleted
  • Resolution set to worksforme
  • Status changed from new to closed

Seems the <i class="fa fa-gamepad"><!-- icon --></i> workaround is sufficient here, also this is now superseded by the block editor.

Closing as worksforme, feel free to reopen if adding this patch still makers sense.

Note: See TracTickets for help on using tickets.