WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 4 years ago

#2348 closed defect (bug) (fixed)

<div>blah<!--more-->blah</div> breaks themes, validation

Reported by: skeltoac Owned by: ryan
Milestone: 2.1 Priority: normal
Severity: normal Version: 2.0
Component: Optimization Keywords: bg|has-patch bg|2nd-opinion
Focuses: Cc:

Description

TinyMCE produces code like that sometimes. If it were a P tag it would only break validation; because we don't nest P tags the browser closes the P when it reaches the next /DIV. However, because themes generally give one DIV per post, nesting goes all awry when a /DIV is missing from within a post. It causes all following posts to nest and, in most themes, indent. The effect is cumulative with multiple split posts, as often occurs on home pages and in archives.

We should fix any nodes broken by the "more" split in get_the_content().

The attached patch makes it possible to force balanceTags to ignore the option so we can use it to fix the split post in get_the_content(). I profiled this in the ZDE and found that balanceTags took a negligible .14-.49ms per split post. It is not run on posts that are not split.

The patch also includes an optimization in get_the_content(), replacing a preg_match() with a strpos().

Attachments (1)

div-balance.diff (1.7 KB) - added by skeltoac 8 years ago.

Download all attachments as: .zip

Change History (7)

skeltoac8 years ago

comment:1 davidhouse8 years ago

I spent some time bouncing ideas off skeltoac in #wordpress, here's the problem as I understand it in other words in case anyone didn't quite get that brief description:

TinyMCE likes to use divs for various pieces of structure. This is fine until you put a more tag in the middle of one, because we trim everything after the more tag sometimes. This means that all nodes open when the more tag comes along will not have closing tags when we strip. That's okay when the open tag is just a P, the browser can cope. But when its a DIV, the entire structure of the theme could and does get messed up.

What Andy's patch does is any time we need to display the trimmed version is to pass said version through balanceTags to make sure this kind of lost-closing-tag doesn't crop up.

comment:2 skeltoac8 years ago

Bump.

If this doesn't go in, we're going to have to start forcing the <!--more--> tag outside any open elements. It'll annoy authors because they won't be able to split block elements such as blockquote.

comment:3 ryan8 years ago

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

(In [3751]) Div balance across more. Props skeltoac. fixes #2348

comment:4 follow-up: enochfung4 years ago

  • Cc enochfung added
  • Component changed from Optimization to Shortcodes
  • Resolution fixed deleted
  • Status changed from closed to reopened
  • Version changed from 2.0 to 2.9.1

Hello,

This bug is still present in WP 2.9.1.

Example)
<div>

your post text
<!--nextpage-->
more text

</div>

When you call <?php the_content(); ?> the last div is missing.

Temporary fix:

<?php the_content(); ?>
</div>

Add an extra </div> in your theme files. No guarantees on other tags.

comment:5 in reply to: ↑ 4 enochfung4 years ago

Replying to enochfung:

Hello,

This bug is still present in WP 2.9.1.

Example)
<div>

your post text
<!--nextpage-->
more text

</div>

When you call <?php the_content(); ?> the last div is missing.

Temporary fix:

<?php the_content(); ?>
</div>

Add an extra </div> in your theme files. No guarantees on other tags.

Actually, you cannot add that extra </div> because it will break your theme on page 2 (page 2 will end up with 2 </div> tags).

comment:6 nacin4 years ago

  • Component changed from Shortcodes to Optimization
  • Resolution set to fixed
  • Status changed from reopened to closed
  • Version changed from 2.9.1 to 2.0

Hi enochfung,
Please open a new ticket instead of opening a ticket that was closed as fixed against a released milestone.

This is actually a related but different bug. This ticket had to do with the <!--more--> tag, not the <!--nextpage--> tag. (I'm not sure if we balance tags across pages, apparently not if you're reporting the bug.)

Feel free to open a new ticket on that and you can certainly refererence #2348 (this one). We have a balanceTags() function that we can utilize to fix this.

Note: See TracTickets for help on using tickets.