Make WordPress Core

Opened 7 years ago

Closed 5 years ago

#39377 closed defect (bug) (duplicate)

wpautop adds a extra </p>

Reported by: pbearne's profile pbearne Owned by: azaozz's profile azaozz
Milestone: Priority: normal
Severity: normal Version: 4.7
Component: Formatting Keywords:
Focuses: Cc:

Description

I am seeing similar problem this code

<?php
wpautop( '<div><span></span><div></div></div>' );

renders

 "<div><span></span></p>
<div></div>
</div>
"

Attachments (4)

Autop-extra-p.php.patch (782 bytes) - added by pbearne 7 years ago.
Autop-extra-p.php.2.patch (780 bytes) - added by pbearne 7 years ago.
fixed the uses comment
formatting.php.patch (1.0 KB) - added by pbearne 7 years ago.
fixes it
autop.patch (1.5 KB) - added by pbearne 7 years ago.
patch and test

Download all attachments as: .zip

Change History (15)

@pbearne
7 years ago

fixed the uses comment

@pbearne
7 years ago

fixes it

#1 @pbearne
7 years ago

  • Keywords has-patch has-unit-tests dev-feedback added

Added a patch that does fix it but the fix is to add span to the block items which does feel right

This was caused by https://core.trac.wordpress.org/changeset/38592

#2 @azaozz
7 years ago

  • Keywords has-patch removed

Looking at formatting.php.patch, we cannot add span to the list of "block tags", it is not a block :) That would introduce numerous failures with spans in paragraphs.

Not even sure if this edge case can be fixed without removing the extra \n from here. We can try adding exceptions for the more common inline tags that are before a block tag where both are inside another block tag, but that will be quite slow and complex, and will have additional edge cases.

This looks quite similar to the nested block tags problems in autop. More cases are tracked in #38656. Perhaps better to close it as duplicate.

#3 @pbearne
7 years ago

I suspected that adding span to the blocks wouldn't be the best fix

Can we keep the unit test?

@pbearne
7 years ago

patch and test

#4 follow-up: @pbearne
7 years ago

  • Keywords has-patch added
  • Summary changed from wpautop add a extra </p> to wpautop adds a extra </p>

I had a quick look at the function again and saw that where are handling <option> to fix the same problem so I have been able to add similar code to strip line breaks before and after <span> elements and this works

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


7 years ago

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


7 years ago

#7 @pbearne
7 years ago

@dd32 can push into the next 4.7.x as well as #39307

#8 @dd32
7 years ago

  • Owner set to azaozz
  • Status changed from new to reviewing

#9 in reply to: ↑ 4 @azaozz
7 years ago

  • Keywords has-unit-tests has-patch removed

Replying to pbearne:

I had a quick look at the function again and saw that where are handling <option> to fix the same problem so I have been able to add similar code to strip line breaks before and after <span> elements

As I said above this will introduce numerous regressions. All spans that are not immediately inside a block element will be affected. Also all <br> around spans will be gone. Couple of cases:
1 <span>2</span> 3 will render as 123 instead of 1 2 3

1 <span>2</span>
3

will render as 123 instead of

1 2
3

I still don't see a good way to fix this apart from reverting [38592].

I had a quick look at the function again and saw that where are handling <option> to fix the same problem

Yes, that works for <option> as it is only allowed in <select> and there can't be any text around it.

Last edited 7 years ago by azaozz (previous) (diff)

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


7 years ago

#11 @pento
5 years ago

  • Keywords dev-feedback removed
  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from reviewing to closed

Closing this as a duplicate of #27350, which has the same root cause.

Note: See TracTickets for help on using tickets.