WordPress.org

Make WordPress Core

Opened 9 years ago

Last modified 3 weeks ago

#9437 reopened feature request

Allow the use of inline SVG in posts

Reported by: codedread Owned by:
Milestone: Future Release Priority: low
Severity: minor Version: 2.5
Component: Formatting Keywords: has-patch needs-refresh needs-testing
Focuses: Cc:

Description

My WordPress 2.5 install serves the application/xhtml+xml MIME type (true XHTML) for the purposes of inlining SVG which I've done on my 'blog chrome'. However, if I try to inline SVG in the contents of a post as follows:

<svg xmlns="http://www.w3.org/2000/svg">

<circle cx="50" cy="50" r="30" fill="blue">

<animateTransform attributeName="transform" type="scale" to="1.5" dur="2s" fill="freeze"/>

</circle>

</svg>

WordPress mangles it into:

<svg xmlns="http://www.w3.org/2000/svg"><br />

<circle cx="50" cy="50" r="30" fill="blue"><br />

<animatetransform attributeName="transform" type="scale" to="1.5" dur="2s" fill="freeze"/><br />

</circle><br />

</svg>

There are two problems with this:

1) WordPress converts the <animateTransform> element into a <animatetransform> element. WordPress should at least preserve the case of the element (whether this is on a whitelist of SVG elements or all elements I don't care). List of SVG elements are here: http://www.w3.org/TR/SVG11/eltindex.html

2) The extra <br /> elements. These are forgivable since the elements are simply ignored by the browser's SVG parser.

Can someone confirm the behavior in WordPress 2.7.x?

Attachments (4)

sem-autolink-uri.php (3.9 KB) - added by Denis-de-Bernardy 8 years ago.
9437.diff (765 bytes) - added by nacin 4 years ago.
9437-1.patch (1.7 KB) - added by jared_smith 3 years ago.
Updated patch and unit test
9437.2.diff (1.9 KB) - added by jared_smith 2 years ago.
Updated patch with code changes and unit tests

Download all attachments as: .zip

Change History (36)

#1 @codedread
9 years ago

Ok, I actually downloaded a local copy of the WordPress trunk and I see that things have changed considerably since WP 2.5.

Now if I edit the HTML tab in the rich editor with:

<svg xmlns="http://www.w3.org/2000/svg">

<circle cx="50" cy="50" r="30" fill="blue">

<animateTransform attributeName="transform" type="scale" to="1.5" dur="2s" fill="freeze"/>

</circle>

</svg>

WordPress now munges it into:

<p><svg xmlns="http://www.w3.org/2000/svg"></p>
<p><circle cx="50" cy="50" r="30" fill="blue"><br />
<animateTransform attributeName="transform" type="scale" to="1.5" dur="2s" fill="freeze"/><br />
</circle><br />
</svg></p>

NOTES:

1) The case is preserved which is a Good Thing.

2) Now HTML paragraph wrappers are placed around each line, which is a Bad Thing since now tags are mis-nested (preventing the rendering of the circle completely, at least before the circle would render).

3) If I collapse all the SVG code into one line (no line breaks) then WordPress wraps it in a paragraph but otherwise leaves the code alone, which is a Good Thing.

So all in all, if I upgrade my WP installation to 2.7 I shouldn't have as much of a problem, but really #2 needs to be fixed.

#2 @hakre
9 years ago

  • Resolution set to wontfix
  • Status changed from new to closed

for 2: There is not a straight source-code editor in WordPress. Even the so called "HTML" Editor is not a true HTML-Editor, as it is more a Text Editor that todes accept HTML Elements as well.

Unfourtionalty you add even something different called SVG that is different to HTML.

Well, it is formatted the same way, but the Editor can not recorgnaize SVG parts.

Because it is not recorgnizing this (and there is nothing giving a clue that WordPress should support SVG [even you and I love it]), this will never be fixed.

BUT, partly good news: It somehow may work in WordPress.

#3 @Denis-de-Bernardy
9 years ago

  • Resolution wontfix deleted
  • Status changed from closed to reopened

@hakre: there could actually be some kind of filter that gets applied to the $allblocks variable in wpautop(). it would allow to extend the elements that should get ignored. reopening for this reason, to get dev-feedback.

@codedread: whatever the lead devs answer, your short term solution is to disable wpautop (there are plugins to do this) and the wysiwyg editor.

#4 @Denis-de-Bernardy
9 years ago

  • Keywords dev-feedback added; xhtml svg removed

#5 @hakre
9 years ago

@denis: yeah, infact, that would be worth a test. just added svg, circle and animateTransform to the list manually and it did not a p- and br-elements any longer:

wp-includes/formatting.php, line ~120 in function wpautop():

$allblocks = '(?:svg|circle|animateTransform|table|thead|tfoot|caption|colgroup|tbody|tr|td|th|div|dl|dd|dt|ul|ol|li|pre|select|form|map|area|blockquote|address|math|style|input|p|h[1-6]|hr)';

anyway, it did not display the SVG to me, but this is because of the doctype of the theme - i have not changed it to the needed xhtml+svg one.

so a filter there would do the job actually.

#6 follow-up: @codedread
9 years ago

This is good news that a filter (which seems like a relatively simple fix) would do the trick. Is this something that could be added to a future version of WordPress? As you (may) know, SVG will be allowed in HTML5 (text/html serialization) and will eventually be supported in browsers (all but one browser currently supports SVG-in-XHTML, application/xhtml+xml serialization).

List of all SVG elements are here: http://www.w3.org/TR/SVG11/eltindex.html

Unfortunately I'm not on WP 2.7 (I'm a couple releases behind). When I migrate to a more recent version I will investigate this some more.

#7 @codedread
9 years ago

  • Cc codedread@… added

#8 @Denis-de-Bernardy
9 years ago

  • Keywords needs-patch added
  • Milestone changed from Unassigned to Future Release
  • Type changed from defect (bug) to feature request

#9 in reply to: ↑ 6 @hakre
9 years ago

Replying to codedread:

List of all SVG elements are here: http://www.w3.org/TR/SVG11/eltindex.html

a|altGlyph|altGlyphDef|altGlyphItem|animate|animateColor|animateMotion|animateTransform|circle|clipPath|color-profile|cursor|definition-src|defs|desc|ellipse|feBlend|feColorMatrix|feComponentTransfer|feComposite|feConvolveMatrix|feDiffuseLighting|feDisplacementMap|feDistantLight|feFlood|feFuncA|feFuncB|feFuncG|feFuncR|feGaussianBlur|feImage|feMerge|feMergeNode|feMorphology|feOffset|fePointLight|feSpecularLighting|feSpotLight|feTile|feTurbulence|filter|font|font-face|font-face-format|font-face-name|font-face-src|font-face-uri|foreignObject|g|glyph|glyphRef|hkern|image|line|linearGradient|marker|mask|metadata|missing-glyph|mpath|path|pattern|polygon|polyline|radialGradient|rect|script|set|stop|style|svg|switch|symbol|text|textPath|title|tref|tspan|use|view|vkern

#10 follow-up: @Denis-de-Bernardy
9 years ago

mmm, that would make the regex larger than 256 chars. Maybe it's a better idea to have wpautop ignore areas enclosed within <svg> tags.

#11 @Denis-de-Bernardy
9 years ago

  • Component changed from General to Formatting

#13 in reply to: ↑ 10 @hakre
9 years ago

Replying to Denis-de-Bernardy:

... Maybe it's a better idea to have wpautop ignore areas enclosed within <svg> tags.

that popped in my mind as well. better way to go that route.

#14 @codedread
9 years ago

Yes, this probably makes more sense because there are some new elements in SVGT 1.2 which at least one browser has begun supporting (textArea, audio, video). I have no idea how hard it would be to get WP to ignore <svg..../svg> so hoping someone has a patch soon :)

#16 @Denis-de-Bernardy
8 years ago

  • Milestone changed from Future Release to 2.9

#17 @Denis-de-Bernardy
8 years ago

  • Keywords dev-feedback removed

#18 @Denis-de-Bernardy
8 years ago

see also #10247 for <video>, <audio> and <source>

#19 @codedread
8 years ago

btw, another option is to only filter on SVG elements/attributes that have camelCase:

Elements:
altGlyph|altGlyphDef|altGlyphItem|animateColor|animateMotion|animateTransform|clipPath|feBlend|feColorMatrix|feComponentTransfer|feComposite|feConvolveMatrix|feDiffuseLighting|feDisplacementMap|feDistantLight|feFlood|feFuncA|feFuncB|feFuncG|feFuncR|feGaussianBlur|feImage|feMerge|feMergeNode|feMorphology|feOffset|fePointLight|feSpecularLighting|feSpotLight|feTile|feTurbulence|foreignObject|glyphRef|linearGradient|radialGradient|textPath

Guess that's still a lot of elements to filter on though.

#20 @Denis-de-Bernardy
8 years ago

@codedread: possibly. the thing here is that we're only hiding the issue under the rug. what's really needed is a proper escaping mechanism for whatever we don't want wpautop to touch.

attaching my latest escape code for reference.

#21 @hakre
8 years ago

+1 for the general analysis regarding wpautop.

#22 @westi
8 years ago

  • Milestone changed from 2.9 to Future Release

No patch moving to Future for now.

#23 @blizzard@…
8 years ago

  • Cc blizzard@… added

#24 @solarissmoke
7 years ago

  • Summary changed from WordPress munges camelcase inline SVG elements to Allow the use of inline SVG in posts

The camelcase issue seems to be resolved now.

However, with browser support for inline SVG now becoming mainstream, this is something that could probably do with being addressed. wpautop shouldn't mess with svg blocks.

#25 follow-up: @hakre
7 years ago

What about having some default shortcode like

[noautop]your svg or whatever x(ht)ml(5) here[/noautop]

in which anything in there gets ignored by wp_autop? Or does that exists already?

#26 in reply to: ↑ 25 @solarissmoke
7 years ago

Replying to hakre:

What about having some default shortcode in which anything in there gets ignored by wp_autop? Or does that exists already?

No.

There is a plugin which lets you insert raw HTML - it searches the_content for a [raw] tag, removes and stores the contents, replacing them with a comment marker, waits for all the other the_content filters to be applied, and then reinserts the raw content (i.e., it doesn't use WP's shortcode API).

@nacin
4 years ago

#27 @nacin
4 years ago

  • Keywords has-patch needs-unit-tests added; needs-patch removed
  • Priority changed from normal to low
  • Severity changed from normal to minor

The camelcase issue seems to be resolved now.

The extra <br /> elements. These are forgivable since the elements are simply ignored by the browser's SVG parser.

Given these two statements, it seems like this ticket is quite minor at this point, as both original points are not blockers for inline SVG use. It seems like the fix here would be to just not process the insides of svg elements in wpautop. Yes? 9437.diff, then. A unit test to back that up would be swell.

#28 @jared_smith
3 years ago

  • Keywords needs-unit-tests removed

I've re-rolled the latest patch and added some unit tests, but it's clear that the fix is more complex than that one-line patch, and is going to require more digging into the code. I'm attaching the updated patch and unit tests that I've been using to show this.

@jared_smith
3 years ago

Updated patch and unit test

#29 @chriscct7
2 years ago

  • Keywords needs-refresh added

#30 @jared_smith
2 years ago

OK, I'm re-tackling this issue at the WordCamp US contributor day.

I'm uploading a re-rolled version of the patch, showing that simply adding 'svg' to the list of tags that wp_autop should ignore doesn't work. (It also doesn't seem to be ignoring the 'script' tag as it should, as shown in the unit tests that are part of the patch.)

Last edited 2 years ago by jared_smith (previous) (diff)

@jared_smith
2 years ago

Updated patch with code changes and unit tests

#31 @swissspidy
10 months ago

  • Keywords needs-testing added

#32 @Boswall
3 weeks ago

Testing as part of WordCamp Manchester 2017 Contributor day.

Fails Tests_Formatting_Autop::test_that_wpautop_ignores_inline_scripts.

-'<p><script type="text/javascript">
-
-				 var dummy = 1;
+'<p><script type="text/javascript"></p>
+<p>				 var dummy = 1;
 			</script></p>'

Passes all other tests in Tests_Formatting_Autop.

Last edited 3 weeks ago by Boswall (previous) (diff)
Note: See TracTickets for help on using tickets.