Make WordPress Core

Opened 10 years ago

Closed 10 years ago

Last modified 8 years ago

#25646 closed defect (bug) (fixed)

figcaption should not be treated as a block by wpautop()

Reported by: laacz's profile laacz Owned by:
Milestone: 3.9 Priority: normal
Severity: normal Version: 3.6.1
Component: Formatting Keywords: needs-patch needs-unit-tests
Focuses: Cc:

Description

Tested against clean Wordpress 3.6.1 with default theme. Result can be seen here: http://dev.laacz.lv/wp-vanilla/?p=4 and in provided attachment.

When composing entry in "text" mode, following generates invalid (or unwanted) markup.

Post content:

<figure>
<img src="http://dev.laacz.lv/wp-vanilla/wp-content/uploads/2013/10/figcaption.jpg" />
<figcaption>Test figure with a test caption (linebreak after &lt;img/&gt;)</figcaption>
</figure>

<figure>
<img src="http://dev.laacz.lv/wp-vanilla/wp-content/uploads/2013/10/figcaption.jpg" /><figcaption>Test figure with a test caption (no linebreak after &lt;img/&gt;)</figcaption>
</figure>

Generated markup (note invalid </p> and unwanted <br/>):

<figure>
<img src="http://dev.laacz.lv/wp-vanilla/wp-content/uploads/2013/10/figcaption.jpg" /></p>
<figcaption>Test figure with a test caption (linebreak after &lt;img/&gt;)</figcaption>
</figure>
<figure>
<img src="http://dev.laacz.lv/wp-vanilla/wp-content/uploads/2013/10/figcaption.jpg" /><br />
<figcaption>Test figure with a test caption (no linebreak after &lt;img/&gt;)</figcaption>
</figure>

Expected output:

<figure>
<img src="http://dev.laacz.lv/wp-vanilla/wp-content/uploads/2013/10/figcaption.jpg" />
<figcaption>Test figure with a test caption (linebreak after &lt;img/&gt;)</figcaption>
</figure>
<figure>
<img src="http://dev.laacz.lv/wp-vanilla/wp-content/uploads/2013/10/figcaption.jpg" /><figcaption>Test figure with a test caption (no linebreak after &lt;img/&gt;)</figcaption>
</figure>

Attachments (3)

figure.jpg (39.2 KB) - added by laacz 10 years ago.
Screenshot of generated HTML.
figcaption-not-block.25646.diff (1.1 KB) - added by oso96_2000 10 years ago.
Remove figcaption from block elements list at wpautop function.
figcaption-not-block-fixed.25646.diff (1.8 KB) - added by oso96_2000 10 years ago.
Patch with both instances (formatting.php and editor.js) modified.

Download all attachments as: .zip

Change History (17)

@laacz
10 years ago

Screenshot of generated HTML.

#1 @nacin
10 years ago

  • Milestone changed from Awaiting Review to 3.9
  • Summary changed from Paragraph and linebreak elements get added when using figure, a, img and figcaption to figcaption should not be treated as a block by wpautop()

Thanks for the report, laacz. Looks like there are two distinct bugs here.

  • figcaption is being treated as a block-level element by wpautop(), and you don't want it to be. figcaption is an interesting element — it must be either the first or last child of figure. Otherwise, figure contains flow content. Figcaption is neither block-like or flow-like in and of itself, it's just an extension of figure. So we should not be treating it as block-level, you are right.

I'm going to tweak the summary of this bug report slightly to reflect a focus on the second bullet.

#2 @nacin
10 years ago

  • Keywords wpautop added

@oso96_2000
10 years ago

Remove figcaption from block elements list at wpautop function.

#3 @oso96_2000
10 years ago

Hello, I don't know if this is ok, but since it's a really simple change (I think) I wanted to step in and try to submit a patch. Let me know if it's ok or I missed something.

Last edited 10 years ago by oso96_2000 (previous) (diff)

#4 @nacin
10 years ago

Hi oso96_2000, figcaption-not-block.25646.diff looks great. Totally okay for you to step in!

A change has to occur in JavaScript as well — a version of wpautop exists there, too. You'll find it in wp-admin/js/editor.js: https://core.trac.wordpress.org/browser/tags/3.8/src/wp-admin/js/editor.js#L163.

@oso96_2000
10 years ago

Patch with both instances (formatting.php and editor.js) modified.

#5 @oso96_2000
10 years ago

Thanks nacir! :D

I missed that one. Uploaded a new patch with both instances modified. Hope it works now.

Version 0, edited 10 years ago by oso96_2000 (next)

#6 @oso96_2000
10 years ago

  • Keywords has-patch needs-testing added

#7 @johnbillion
10 years ago

In 27527:

figcaption should not be treated as a block-level element by wpautop(). See #25646. Props oso96_2000.

#8 follow-up: @johnbillion
10 years ago

  • Keywords wpautop has-patch needs-testing removed

[27527] helped this on its way but has not completed fixed the issue. A <br> is still inserted within <figure>, however this occurs regardless of whether there's a <figcaption> in there or not.

Is there a way we can treat <figure> like <pre> and treat its contents as-is?

#9 in reply to: ↑ 8 @nacin
10 years ago

Replying to johnbillion:

[27527] helped this on its way but has not completed fixed the issue. A <br> is still inserted within <figure>, however this occurs regardless of whether there's a <figcaption> in there or not.

Is there a way we can treat <figure> like <pre> and treat its contents as-is?

I'm not sure contents should be treated as-is... Wouldn't a line break be preferred in some instances?

Some unit tests to explain expected and actual (when figcaption is present and isn't) would go a long way to fixing this.

This ticket was mentioned in IRC in #wordpress-dev by nacin. View the logs.


10 years ago

#11 @johnbillion
10 years ago

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

Follow-up: #27559.

#12 @wonderboymusic
10 years ago

In 28853:

Fix wpautop() unit tests. See #25646, #22230, #25856.

Props rachelbaker.
Fixes #28638.

#13 @rcgoncalves
9 years ago

This bug seems to be back in WordPress 4.2.2.

I checked the source code, and it seems that [27527] was reverted.

#14 @bkahlert
8 years ago

  • Keywords needs-patch needs-unit-tests added

I can conform that the bug was reintroduced.
You might wanna fix it again and do regression testing.

Note: See TracTickets for help on using tickets.