WordPress.org

Make WordPress Core

Opened 9 years ago

Closed 3 years ago

#1706 closed defect (bug) (fixed)

wpautop() not stripping <br /> tags before <p withattributes>

Reported by: rockinfree Owned by: mdawaffe
Milestone: 2.0.6 Priority: normal
Severity: normal Version: 2.3
Component: Formatting Keywords:
Focuses: Cc:

Description

<hr /><p>blah
blah
blah</p>
<hr /><p class="center">blah
blah
blah</p>

Entered through the wp-admin/post.php page should become:

	<hr />

<p>blah<br />
blah<br />
blah</p>
	<hr />
<p class="center">blah<br />
blah<br />
blah</p>

On the generated page (after being processed by wpautop()), but instead becomes:

	<hr />

<p>blah<br />
blah<br />
blah</p>
	<hr /><br />
<p class="center">blah<br />
blah<br />
blah</p>

(note the extra <br /> after the second <hr /> tag)

The error is caused in functions-formatting.php wpautop():

$pee = preg_replace('!<br />(\s*</?(?:p|li|div|dl|dd|dt|th|pre|td|ul|ol)>)!', '$1', $pee);

should be:

$pee = preg_replace('!<br />(\s*</?(?:p|li|div|dl|dd|dt|th|pre|td|ul|ol)[^>]*>)!', '$1', $pee);

(Note that the original regexp is missing the [>]*, which is present for all other patterns in this function, and is necessary to allow, for example, <p class="center"> as in the repro case.)

Attachments (2)

functions-formatting.php (32.7 KB) - added by rockinfree 9 years ago.
Fixed version of file with line 75 in wpautop() updated (sorry, no diff on this system)
functions-formatting.php.diff (994 bytes) - added by markjaquith 9 years ago.
patch for WP 1.6 SVN

Download all attachments as: .zip

Change History (18)

rockinfree9 years ago

Fixed version of file with line 75 in wpautop() updated (sorry, no diff on this system)

markjaquith9 years ago

patch for WP 1.6 SVN

comment:1 markjaquith9 years ago

  • Keywords bg|has-patch bg|commit added
  • Owner changed from anonymous to markjaquith
  • Status changed from new to assigned

Thanks! Patch added.

Excellent bug report, by the way. Much appreciated.

comment:2 ryan8 years ago

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

(In [4167]) Recognize paragraph tags with attributes when stripping breaks. Props rockinfree. fixes #1706

comment:3 ryan8 years ago

(In [4240]) Recognize paragraph tags with attributes when stripping breaks. Props rockinfree. fixes #1706

comment:4 foolswisdom8 years ago

  • Milestone set to 2.0.5

comment:5 anonymous8 years ago

  • Milestone 2.0.5 deleted

Milestone 2.0.5 deleted

comment:6 foolswisdom7 years ago

  • Milestone set to 2.4 (next)
  • Resolution fixed deleted
  • Status changed from closed to reopened
  • Version changed from 1.5.2 to 2.3

This actually created awkward behavior, which I have encountered working with numerous customers.

Most HTML includes line breaks through out it to easy readability. If within paragraph tags, the best assumption is that new lines <br />s will be explictly defined.

comment:7 foolswisdom7 years ago

  • Owner changed from markjaquith to mdawaffe
  • Status changed from reopened to new

comment:8 westi6 years ago

  • Keywords needs-patch added; bg|has-patch bg|commit removed

comment:9 mdawaffe6 years ago

  • Milestone changed from 2.5 to 2.6

comment:10 Denis-de-Bernardy5 years ago

  • Component changed from Administration to Formatting

comment:11 hakre5 years ago

Please close as wontfix. Fixing things will break other things and vice-
versa. I do not know a single serious developer that wants to touch this.

Best would be to get a binding description first what must (not)/should (not)/can
(not) be done by wpautop. Then testcases must be written (executeable ones) and then the function can be developed accordingly.

related: #2833

comment:12 follow-up: westi5 years ago

  • Keywords needs-unit-tests added; br wpautop functions-formatting.php needs-patch removed
  • Milestone changed from 2.9 to Future Release

Agreed we need comprehensive autop test cases before touching the code in this way.

Moving to Future.

comment:13 hakre5 years ago

Related: #4857

comment:14 in reply to: ↑ 12 hakre5 years ago

Replying to westi:

Agreed we need comprehensive autop test cases before touching the code in this way.

Moving to Future.

Does this mean you agree with "not a single serious developer wants to touch this" as well?

comment:15 hakre4 years ago

Related: #2813

comment:16 nacin3 years ago

  • Keywords needs-unit-tests removed
  • Milestone changed from Future Release to 2.0.6
  • Resolution set to fixed
  • Status changed from new to closed

Most HTML includes line breaks through out it to easy readability. If within paragraph tags, the best assumption is that new lines <br />s will be explictly defined.

This is not related to the initial bug report, nor is it something that we would ever address at this point, as automatic line breaks are integral to wpautop.

Considering this fixed on 2.0.

Note: See TracTickets for help on using tickets.