WordPress.org

Make WordPress Core

Opened 18 months ago

Closed 13 months ago

Last modified 10 months ago

#25646 closed defect (bug) (fixed)

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

Reported by: laacz Owned by:
Milestone: 3.9 Priority: normal
Severity: normal Version: 3.6.1
Component: Formatting Keywords:
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 18 months ago.
Screenshot of generated HTML.
figcaption-not-block.25646.diff (1.1 KB) - added by oso96_2000 15 months ago.
Remove figcaption from block elements list at wpautop function.
figcaption-not-block-fixed.25646.diff (1.8 KB) - added by oso96_2000 15 months ago.
Patch with both instances (formatting.php and editor.js) modified.

Download all attachments as: .zip

Change History (15)

@laacz18 months ago

Screenshot of generated HTML.

comment:1 @nacin15 months 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.

comment:2 @nacin15 months ago

  • Keywords wpautop added

@oso96_200015 months ago

Remove figcaption from block elements list at wpautop function.

comment:3 @oso96_200015 months 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 15 months ago by oso96_2000 (previous) (diff)

comment:4 @nacin15 months 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_200015 months ago

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

comment:5 @oso96_200015 months ago

Thanks nacin! :D

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

Last edited 15 months ago by oso96_2000 (previous) (diff)

comment:6 @oso96_200015 months ago

  • Keywords has-patch needs-testing added

comment:7 @johnbillion13 months ago

In 27527:

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

comment:8 follow-up: @johnbillion13 months 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?

comment:9 in reply to: ↑ 8 @nacin13 months 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.

comment:10 @ircbot13 months ago

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

comment:11 @johnbillion13 months ago

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

Follow-up: #27559.

comment:12 @wonderboymusic10 months ago

In 28853:

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

Props rachelbaker.
Fixes #28638.

Note: See TracTickets for help on using tickets.