WordPress.org

Make WordPress Core

Opened 5 years ago

Last modified 19 months ago

#10269 reopened defect (bug)

wysiwyg bug with shortcodes

Reported by: Denis-de-Bernardy Owned by:
Milestone: Future Release Priority: low
Severity: minor Version: 2.8
Component: Formatting Keywords: has-patch wpautop close
Focuses: Cc:

Description

When inserting two consecutive shortcodes on separate lines, the wysiwyg editor will change the post's contents on save.

[caption /]

[caption /]

gets turned into:

[caption /] [caption /]

the odd thing is that only genuine shortcodes get changed, too. the following is left alone:

[notashortcode /]

[notashortcode /]

Attachments (1)

10269.diff (695 bytes) - added by Denis-de-Bernardy 5 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Denis-de-Bernardy5 years ago

  • Component changed from Shortcodes to Formatting
  • Keywords needs-patch added

it's related to this line in wpautop:

$pee = preg_replace('/<p>\s*?(' . get_shortcode_regex() . ')\s*<\/p>/s', '$1', $pee); // don't auto-p wrap shortcodes that stand alone

comment:2 Denis-de-Bernardy5 years ago

  • Component changed from Formatting to TinyMCE
  • Keywords wpautop added
  • Owner set to azaozz

Moving this to TinyMCE, because the issue really belongs in there. When loading a post with shortcodes in TinyMCE, we should actually add the needed paragraphs in such an event, to ensure that things remain properly formatted.

comment:3 azaozz5 years ago

  • Component changed from TinyMCE to Formatting
  • Owner azaozz deleted
  • Priority changed from normal to low
  • Severity changed from normal to minor

This is clearly a minor wpautop issue (not even sure if it needs fixing), and has nothing to do with TinyMCE.

Filtering the content before loading it in TinyMCE is only done with the php version of wpautop, any other filtering would open a huge can of worms. And what about people that only use the html editor or a different visual editor?

comment:4 Denis-de-Bernardy5 years ago

  • Milestone 2.9 deleted
  • Resolution set to wontfix
  • Status changed from new to closed

k... sounds like a wont ever fix to me... :-)

comment:5 dd325 years ago

  • Milestone set to Future Release
  • Resolution wontfix deleted
  • Status changed from closed to reopened
  • Version set to 2.8

Doesnt sound like a wontfix to me.

comment:6 follow-up: dd325 years ago

While its a minor issue, I dont think its something that should just be closed off as "its only a small bug. dont worry about it"

I'm not sure what the deal should be with this, I mean, Having 2 shortcodes after one another is a completely legit setup, and i'm not sure if they should have to auto-wrap themselves in a <p>.. Afterall, the user may want to use it inline.

BUT the shortcode may already contain a block element, such as a div.. So it needs some more thought i think. According to Norman- on IRC, It worked line this fine under 2.7..

comment:7 Denis-de-Bernardy5 years ago

  • Milestone changed from Future Release to 2.9

cool. well, the fix, if any, is to wrap standalone shortcodes into <div> tags.

Denis-de-Bernardy5 years ago

comment:8 Denis-de-Bernardy5 years ago

  • Keywords has-patch added; needs-patch removed

untested patch attached

comment:9 in reply to: ↑ 6 azaozz5 years ago

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

Replying to dd32:

I'm not sure what the deal should be with this, I mean, Having 2 shortcodes after one another is a completely legit setup, and i'm not sure if they should have to auto-wrap themselves in a <p>.. Afterall, the user may want to use it inline.

Exactly, shortcodes shouldn't be force-wrapped in block elements. Think there was a long discussion about a year ago whether to process shortcodes before or after wpautop which is related to this.

It doesn't make any difference how they are displayed in the editor as putting a shortcode on a new line wouldn't wrap it in <p> anyways and this is the correct behavior.

comment:10 webprodigy4 years ago

  • Milestone changed from 2.9 to 3.0
  • Resolution wontfix deleted
  • Status changed from closed to reopened

There is still an issue here I think. Even if I wrap the shortcode in <p> elements, all <p> tags are stripped and the two shortcodes appear side-by-side. This is fine if I know to wrap them in div's, but for the average user trying to lay things out, this is definitely a problem.

comment:11 nacin4 years ago

  • Milestone changed from 3.0 to Future Release

comment:12 MikeHansenMe20 months ago

I agree with azaozz. If the shortcode was wrapped in autop and then the content of the shortcode was "<div>hello</div>" you would end up with "<p><div>hello</div></p>" which is incorrect markup. I think to wrap the shortcode in p tags would create more problems than it would fix.

comment:13 MikeHansenMe20 months ago

  • Keywords close added

comment:14 MikeHansenMe19 months ago

  • Cc mdhansen@… added
Note: See TracTickets for help on using tickets.