Unbalanced tags across more and nextpage tags

[tellyworth]

It's easy to produce broken front page markup by including a --nextpage-- tag that breaks an enclosing bold or italic tag. There's some code in get_the_content that fixes this for --more-- tags, but it doesn't handle --nextpage--, and it'd be more efficient to do it at post save time.

The enclosed patch fixes this by splitting the content into slices at those boundaries and separately balancing each slice. Balancing happens in the content_save_pre action. No filtering is needed on the output side for posts saved after this filter.

It was a bit of a struggle figuring out where to fit this but I think the solution is fairly clean. It includes a new split_nextpage() function that can be used instead of ad-hoc regexps for splitting a post into pages.

[tellyworth]

By "No filtering is needed on the output side" I mean no _balance_ filtering of course.

[tellyworth]

Unit tests are in ​http://svn.automattic.com/wordpress-tests/wp-testcase/test_post_filtering.php.

[Denis-de-Bernardy]

ah, yes, the number of sidebar gets knocked down issues that this might fix...

[Denis-de-Bernardy]

broken patch

[Denis-de-Bernardy]

see: #9765

[Denis-de-Bernardy]

fixed in r11398

[nacin]

Was not fixed. Unit tests are still broken. Closed #20278 as a duplicate.

[devesine]

Bringing over patch from #20278:

The attached patch splits up the text (using a regex patterned after the regexes used to split out quicktags in other areas such as get_the_content()) runs force_balance_tags against each part, and glues them back together.

This could be done in force_balance_tags instead, but my first thought is that force_balance_tags is a lower-level function; since it ignores options (use_balanceTags) and has no option to not balance (truism), it should ignore other WordPress-specific conceits like quicktags. Among other things, there's no expectation that it's always being used for content that should respect those quicktags (it's also used on the comment_text filter and in get_the_content() after the more and nextpage quicktags have been handled).

[SergeyBiryukov]

Closed #24191 as a duplicate.

[mdawaffe]

props go to devesine

[mdawaffe]

attachment:balanceTags_quicktags-trunk.patch​​ looks solid to me. Refreshed for trunk in attachment:6297.diff​.

Passes all relevant unit tests including the ones that are skipped for this ticket:

phpunit --group 6297 tests/post/filtering.php

I don't like the "quicktags" naming. Maybe "balance_tags_delimiters"? Maybe roll the <!-- and --> into the values returned by the filter?

[devesine]

Replying to mdawaffe:

I don't like the "quicktags" naming. Maybe "balance_tags_delimiters"?

That seems better - when I originally wrote the patch, I mistakenly gathered that 'quicktags' was the general name of comment tags with WP functionality (<!--more--> and <!--nextpage-->) rather than the name of the button bar in the editor.

Maybe roll the <!-- and --> into the values returned by the filter?

Ah, to provide more flexibility in delimiters?

Something like this, if I understand your suggestions correctly:

$balance_tags_delimiters = apply_filters( 'balance_tags_delimiters', array( '<!--more.*?-->', '<!--nextpage-->' ) );  
// Capture lets PREG_SPLIT_DELIM_CAPTURE return the delimiters 
$delimiters_regex = '/(' . implode( '|', $balance_tags_delimiters ) . ')/'; 
$parts = preg_split( $delimiters_regex, $text, -1, PREG_SPLIT_DELIM_CAPTURE ); 

[mdawaffe]

Replying to devesine:

Maybe roll the <!-- and --> into the values returned by the filter?

Ah, to provide more flexibility in delimiters?

Yes. Maybe it makes sense to split on some custom shortcode, for example. I'm not sure.

Something like this, if I understand your suggestions correctly:

Yes. That's what I had in mind. Again, I'm not positive it's better, but I think it probably is :)

[devesine]

Replying to mdawaffe:

Again, I'm not positive it's better, but I think it probably is :)

Seems entirely reasonable.

attachment:6297-renamed.diff​ has those changes now.

[nacin]

Looks good. The filter will need to be commented. Let's get this in early 3.8.

[wonderboymusic]

In 26050:

Split the content to balance open tags when <!--nextpage--> and <!--more.*?--> are used. Needs filter inline docs. The 4 unit tests that were previously failing for ticket 6297 now pass.

See #6297.
Props devesine.

[Hanni]

Looks like @simonweatley has taken care of the docs in #25518.

[mdawaffe]

On further inspection, the behavior here is a little odd.

For nextpage, it makes sense to fix the markup on save to ensure the markup of each page is sane.

For more, it doesn't. For example, for the following:

<blockquote>
Above
<!--more-->
Below
</blockquote>

On an index page, I would expect to see either:

<blockquote>
Above
<a href="#more-123">...</a>
</blockquote>

or maybe:

<blockquote>
Above
</blockquote>
<a href="#more-123">...</a>

In 3.7.1, I see the former. In [26050], I see the latter. I'm not sure which is "better", but the difference is not backward compatible.

More importantly, on a permalink page, I would expect to see:

<blockquote>
Above
<a id="more-123></a>
Below
</blockquote>

That's what I see in 3.7.1. In [26050], though, I see:

<blockquote>
Above
</blockquote>
<a id="more-123></a>
Below

Which is unexpected.

For nextpage, each piece of text is only ever visible on one page, so fixing each page's markup individually makes sense.

For more, some pieces of text are shown in two different contexts, so fixing the markup "before the jump" independently from the markup "after the jump" doesn't quite work.

Idea: Balance tags on save only by page. Balance tags of index page's pre-more-markup on display.
Cons:

• More work on display.
• Using a filter on display that plugins may expect to only be used on save?
• Toggling the setting changes the display of already written posts.

[nacin]

In 26557:

Revert [26050], stop trying to balance open HTML tags across nextpage and more tags for now.

see #6297.

[MikeHansenMe]

see #30284