Make WordPress Core

Opened 8 years ago

Last modified 7 years ago

#34722 new defect (bug)

Open P Tag in shortcode related to h tag

Reported by: backups's profile BackuPs Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.4
Component: Formatting Keywords: 2nd-opinion has-patch needs-testing
Focuses: Cc:

Description

Hi Wordpress core Team

There is a bug in wordpress.

When you add a simple text in the editor like this

<h3>Integer in ex vel urna tempor ultrices.</h3> Morbi vehicula a orci nec dignissim.

In the front of the website and in the page source code this text is changed by wordpress to this and there are no issues or open or extra p-tags.

<h3><p>Integer in ex vel urna tempor ultrices.</h3> <p>Morbi vehicula a orci nec dignissim.</p>

http://i.imgur.com/9TCF7gi.png

However this does not happen in a shortcode

F.e. if you create a simple shortcode to add a div with a class around your text. The result is open and or extra p-tags and break tags,

<?php
function theme_shortcode_div($atts, $content = null, $code) {

        $content = $content;
        

        return '<div class="theme-div">' . $content .'</div>';
}

add_shortcode('theme_div', 'theme_shortcode_div');

And add the text within a sshortcode

[theme_div]<h3>Integer in ex vel urna tempor ultrices.</h3> Morbi vehicula a orci nec dignissim. [/theme_div]

The same text is rendered like this.

<div class="theme-div"><br />
<h3>Integer in ex vel urna tempor ultrices.</h3>
<p> Morbi vehicula a orci nec dignissim. </div>

The p tag is never closed and before the h-tag there is suddenly a break tag. I tested this in any of the default themes and i dont know how many commercial themes and all with same results.

The p tag is left open and there is a extra break tag.

http://i.imgur.com/52p291S.png

It can even gets worse if you add it like this and embed your text in a p-tag.

[theme_div]<h3><p>Integer in ex vel urna tempor ultrices.</h3> Morbi vehicula a orci nec dignissim.</p> [/theme_div]

Now the end result is this

<div class="theme-div"><br />
<h3>
<p>Integer in ex vel urna tempor ultrices.</h3>
<p> Morbi vehicula a orci nec dignissim.</p>
<p> </div>

There is suddenly a extra br tag before the h-tag a extra p tag suddenly in the h-tag and a extra p tag at the end of the div.

http://i.imgur.com/J3597aM.png

Note: All code has been added in text mode of the editor. The text was added as shown. H-tag and normal text without any break tag all on one line

Note: used theme in my images twenty sixteen.

Note: Shortcode added to the functions.php exactly as shown above.

Note: You can only see the p-tags missing in the source of the page. If you inspect the page in firebug or chrome it shows the closing p-tags as that is what browsers do. They try to close the p-tags by default even if it is missing.

http://i.imgur.com/aTwn2Wg.png

Please resolve as this is a really annoying bug.

Attachments (1)

34722.diff (1.8 KB) - added by MattyRob 8 years ago.

Download all attachments as: .zip

Change History (24)

#1 @BackuPs
8 years ago

I forgot to mention.

Note: If you dont add a H-tag. There is no issue at all and all is ok.

Note: there is a mistake in my above html... Somehow a extra p tag got in after the starting h-tag. Nevertheless removing that does still leave me with the open and unclosed p-tag issue when a shortcode is used. Sorry about my typo in the html code.

This

[theme_div]<h3><p>Integer in ex vel urna tempor ultrices.</h3> Morbi vehicula a orci nec dignissim.</p> [/theme_div]

should have been this

[theme_div]<h3>Integer in ex vel urna tempor ultrices.</h3><p>Morbi vehicula a orci nec dignissim.</p> [/theme_div]
Last edited 8 years ago by BackuPs (previous) (diff)

#2 @swissspidy
8 years ago

  • Component changed from TinyMCE to Formatting

#3 follow-up: @BackuPs
8 years ago

More notes and information to add to narrow it down.

1) If i add the pure htm code with a div around it manually without using the shortcode it also ok and wordpress formattes correctly the p-tag by closing it.

<div class="theme-div"><h3>Integer in ex vel urna tempor ultrices.</h3> Morbi vehicula a orci nec dignissim.</div>

Which leads me to believe this is shortcode related and not tinymce. But i am not sure about that.

2) There is a mistake in above html provided. The gets worse should be

[theme_div]<h3>Integer in ex vel urna tempor ultrices.</h3><p>Integer in ex vel urna tempor ultrices.Morbi vehicula a orci nec dignissim.</p>[/theme_div]

3) Of course our shortcode does more then above. I just narrowed it down to a basic one.

The $content=$content; is usesless in our provided code. But simply adding do_shortcode($content); does not change the result. So the code was provided basic. That line could have been removed from the provide code or f.e. changed to this.

<?php
function theme_shortcode_div($atts, $content = null, $code) {

        $content = do_shortcode($content);
        

        return '<div class="theme-div">' . $content .'</div>';
}

add_shortcode('theme_div', 'theme_shortcode_div');

I hope this narrows it down.

Last edited 8 years ago by BackuPs (previous) (diff)

#4 in reply to: ↑ 3 ; follow-up: @swissspidy
8 years ago

  • Keywords close added

First of all, shortcodes are parsed after wpautop() and wptexturize() post formatting has been applied. This means that your shortcode output HTML won't automatically have curly quotes applied, p and br tags added, and so on. If you do want your shortcode output to be formatted, you should call wpautop() or wptexturize() directly when you return the output from your shortcode handler.

Second, inserting

<h3>Integer in ex vel urna tempor ultrices.</h3> Morbi vehicula a orci nec dignissim.

correctly outputs

<h3>Integer in ex vel urna tempor ultrices.</h3><p> Morbi vehicula a orci nec dignissim.</p>

on my local 4.4 site (without any shortcodes).

So what you experience is actually the documented, expected behaviour. I don't see a bug here.

#5 @BackuPs
8 years ago

Hi

Why should i call wpautop() or wptexturize() as when the shortcode call adds the p-tag and does not close it.

I know this returns a perfect html as that is what i wrote in the first place.

<h3>Integer in ex vel urna tempor ultrices.</h3> Morbi vehicula a orci nec dignissim.

But that same code embedded in a shortcode does not and returns

<h3>Integer in ex vel urna tempor ultrices.</h3> <p>Morbi vehicula a orci nec dignissim.

Moreover if i dont add h3 (or any h-tag) it returns

Integer in ex vel urna tempor ultrices. Morbi vehicula a orci nec dignissim.

No p-tag added at all. The open p-tag only is added when a h3 tag is in the text.

Given this shortcode. What should be changed to make it work correctly? I just want a div with a class around my text so i can add css to manipulate. Why is the shortcode routine altering the text and adds a open p-tag? I believe if it alters it it should so it correctly.

<?php
function theme_shortcode_div($atts, $content = null, $code) {

        $content = do_shortcode($content);
        

        return '<div class="theme-div">' . $content .'</div>';
}

add_shortcode('theme_div', 'theme_shortcode_div');

I still believe this is a bug. Please provide the solution for it or the correct shortcode.

Thank you

#6 @BackuPs
8 years ago

Note

Try

[theme_div]<h3>Integer in ex vel urna tempor ultrices.</h3>Integer in ex vel urna tempor ultrices. Morbi vehicula a orci nec dignissim.[/theme_div]
<?php
function theme_shortcode_div($atts, $content = null, $code) {

        $content = do_shortcode($content);
        $content=wpautop( $content );
        

        return '<div class="theme-div">' . $content .'</div>';
}

add_shortcode('theme_div', 'theme_shortcode_div');

You will notice that the p-tags are close but i still get a empty p-tag -right before the h-tag and just after the div opens.... That should not be there at all. Moreover what if the text added also has advanced shortcodes to present a bloglist, portfolio grid, image grid etc. Calling then wpautop() could break the whole code. It is not a good solution i believe.

http://i.imgur.com/TLTW6bH.png

<div class="theme-div"><p></p>
<h3>Integer in ex vel urna tempor ultrices.</h3>
<p>Integer in ex vel urna tempor ultrices. Morbi vehicula a orci nec dignissim.</p>
</div>

Note: The result you see here is in the twenty-16 theme. The default wordpress theme.

Last edited 8 years ago by BackuPs (previous) (diff)

#7 in reply to: ↑ 4 @diegocanal
8 years ago

Replying to swissspidy:

First of all, shortcodes are parsed after wpautop() and wptexturize() post formatting has been applied. This means that your shortcode output HTML won't automatically have curly quotes applied, p and br tags added, and so on. If you do want your shortcode output to be formatted, you should call wpautop() or wptexturize() directly when you return the output from your shortcode handler.

Second, inserting

<h3>Integer in ex vel urna tempor ultrices.</h3> Morbi vehicula a orci nec dignissim.

correctly outputs

<h3>Integer in ex vel urna tempor ultrices.</h3><p> Morbi vehicula a orci nec dignissim.</p>

on my local 4.4 site (without any shortcodes).

So what you experience is actually the documented, expected behaviour. I don't see a bug here.

Hi @swissspidy ,

It really gives the impression you have not fully read @BackuPs description of the issue as she/he describes it in detail with a crystal-clear explanation and your answer has nothing to do with it. Please, carefully read it again and you will see what she/he means.

I am experiencing the same issue and it would be great if someone could give a thoughtful response to it.

Best Regards,
Diego

Last edited 8 years ago by diegocanal (previous) (diff)

#8 @BackuPs
8 years ago

Hi

Can we get a reply on this and confirmation of the issue.

Best regards,
BackuPs

#9 @swissspidy
8 years ago

#34787 was marked as a duplicate.

#10 follow-up: @BackuPs
8 years ago

Note : Themeforest does not allow the use of wpautop() !!!

#11 in reply to: ↑ 10 @StephenCronin
8 years ago

Replying to BackuPs:

Note : Themeforest does not allow the use of wpautop() !!!

Disclaimer: I work for Envato as Quality Specialist (Themes & Plugins)

ThemeForest does not allow themes to remove or modify filters where the wpautop is run, because that can break plugins. For example, you can't remove the filter that applies wpautop to the_content. You should be able to apply wpautop to the content in your shortcode as @swissspidy suggests, as it won't affecting anything else.

However, I don't believe wpautop is going to help you in this case.

To work out what's really going on here, disable your shortcode code, but leave the shortcode in the post. You'll then see this in the source:

<p>[theme_div]<br />
<h3>This is a test heading</h3>
<p> This is the test body [/theme_div]</p>

When you look at the content inside the shortcode, it's exactly the same as what's output when the shortcode is active (ie has the br and doesn't close the p).

I *think* that when wpautop is applied to the_content it should ignore anything within shortcodes, but it seems that's not working in this case.

I'm *guessing* that when h3 is in the shortcode content, wpautop sees the h3 first, sees it's not on a new line and adds the br, but then it no longer sees it as a shortcode, so it goes ahead and applies the p tags, etc.

There's a lot of guessing going on here, sorry, but I don't have time to dig into core right now. I figure I should leave my guesses here in case they can help someone else investigate further.

Note to others: You need to View Source rather than use Firebug or Chrome Developer Tools, as they show you the source after the browser has compensated for the malformed HTML. For example, the browser will close the p tag that was left open, so you won't see that in Firebug or Developer Tools.

#12 @BackuPs
8 years ago

Hi

You are right that removing the shortcode gives this as a result.

<p>[theme_div]<br />
<h3>Integer in ex vel urna tempor ultrices.</h3>
<p>Integer in ex vel urna tempor ultrices. Morbi vehicula a orci nec dignissim.[/theme_div]</p>

But how does that help and solve this issue? How to get the wordpress core team to fix this? As i think this needs to be resolved as it is a bug.

Could we at least confirmation this and put it on the buglist in order to be fixed ?

Last edited 8 years ago by BackuPs (previous) (diff)

#13 @StephenCronin
8 years ago

I've looked into this a little further and it's complicated.

Shortcode content has wpautop applied to it, then shortcode_unautop applied to it which undoes the wpautop changes. But in this case, shortcode_unautop doesn't roll back all the changes made in wpautop.

The wpautop function has one line which adds "a single line break above block-level opening tags" and another line which adds "a double line break below block-level closing tags". If I comment out those lines, I get this:

<div class="theme-div"><h3>This is the test heading</h3> This is the test body </div>

So they are the lines which are behind the extra br and the unclosed p tag. They are necessary for non shortcode content, but they should be rolled back for shortcodes by shortcode_unautop and it's not currently doing that. I'm not a Regex ninja, so I'm not sure how to change shortcode_unautop to do this.

But....

Turns out there are quite a few tickets out there about shortcodes and wpautop, which are at least related to this (if not exactly the same). One of them is 5 years old and has a patch. This one is 8 years old and is "slated for 4.5 Milestone" as part of @miqrogroove 's roadmap proposal for an alternative shortcode syntax.

So, I would expect this to get fixed at some point, maybe 4.5, but as part of the larger reworking of shortcodes.

In the meantime, it's hacky, but you could use this:

function theme_shortcode_div( $atts, $content = null, $code ) {
	$content = str_replace( '<p></p>', '', wpautop( $content) );
	return '<div class="theme-div">' . $content . '</div>';
}
add_shortcode( 'theme_div', 'theme_shortcode_div' );

This applies wpautop to the shortcode content, adding the missing closing p tag and converting the br tag to <p></p> and then manually strips that out.

Of course, you probably want to test that to make sure it works for you. It should fix this particular problem, but may cause other problems with different markup in the shortcode content - I haven't tested it with anything else.

#14 @BackuPs
8 years ago

Hi

Thank you for the feedback and extra info as that is really appreciated. The example given was simplified to narrow down the issue and pin it to the shortcode processor within the wordpress code.

Our real shortcode is much more complicated and has more shortcodes within the shortcode even nested ones of the same type.

I think i might be better of for now leaving the p tag open. However it would be a great plus if the wordpress team fixes this ones and for all. Especially given the fact how long this 'bug' is already existing in the wp core files.

I will try your solution though on the more simple shortcodes we are using and also apply the do_shortcode to the content first in order to see what happens when shortcodes have shortcodes within the content.

<?php
function theme_shortcode_div($atts, $content = null, $code) {
        $content = str_replace( '<p></p>', '', wpautop( do_shortcode($content)) );
        return '<div class="theme-div">' . $content .'</div>';
}

add_shortcode('theme_div', 'theme_shortcode_div');

I can imagine that that will cause many issues though. The open p-tag should not have been there at all in the first place. But that we already agreed upon.

Have a nice weekend ! & Thanks again for the feedback

This ticket was mentioned in Slack in #core by chriscct7. View the logs.


8 years ago

This ticket was mentioned in Slack in #core by chriscct7. View the logs.


8 years ago

#17 @MattyRob
8 years ago

  • Keywords reporter-feedback added

I found this ticket from a comment at https://make.wordpress.org/core/

Following code in a plugin to register the shortcode:

function theme_shortcode_div( $atts, $content = null, $code ) {
	return '<div class="theme-div">' . $content .'</div>';
}
add_shortcode( 'theme_div', 'theme_shortcode_div' );

Test on an admin page with the following code inserted into the admin page:

$content = "[theme_div]<h3>Integer in ex vel urna tempor ultrices.</h3> Morbi vehicula a orci nec dignissim.[/theme_div]";
var_dump( do_shortcode( $content ) );

Made sure to view the source in FF 48.0 (not Firebug which can autocorrect some malformed HTML, output as follows:

string(114) "<div class="theme-div"><h3>Integer in ex vel urna tempor ultrices.</h3> Morbi vehicula a orci nec dignissim.</div>"

No malformed <p> tags anywhere for me on this testing - could it be a plugin conflict perhasp as it seems to work as expected for me in WordPress 4.5.3.

Last edited 8 years ago by MattyRob (previous) (diff)

#18 @BackuPs
8 years ago

Hi

There is no plugin conflict. The issue was tested on a clean wordpress install with the default theme and no plugins and still exist. We dont use firebug we just view the source. The p tag is not closed.

The issue is visible in any browser not only firefox.

Its already been confirmed as a malfunctioning !

Best regards,
BackuPs

Last edited 8 years ago by BackuPs (previous) (diff)

#19 @MattyRob
8 years ago

  • Keywords needs-patch 2nd-opinion added; close reporter-feedback removed

Bug confirmed on my testing - the issue is coming from somewhere in the wpautop() function as removing that resolves the issue.

I haven't found where yet so need some more eyes on this.

This ticket was mentioned in Slack in #core by mattyrob. View the logs.


8 years ago

#21 @MattyRob
8 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

I've had a look through wpautop and the issue is in the handling of HTML within Shortcodes. I though a good way to fix is to wrap shortcode open and close tags in line breaks to allow paragraphing and then strip it back out again after.

Seem sot work in my testing but the Regex needs a second check by someone with more skill. Also needs some more testing of double bracket shortcodes. (tag]sontent[[/tag?)

@MattyRob
8 years ago

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


7 years ago

This ticket was mentioned in Slack in #core by backups. View the logs.


7 years ago

Note: See TracTickets for help on using tickets.