WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#24109 closed defect (bug) (fixed)

Twenty Thirteen: Remove unneeded post formats `add_theme_support()` declaration

Reported by: DrewAPicture Owned by: lancewillett
Milestone: 3.6 Priority: normal
Severity: normal Version: 3.6
Component: Bundled Theme Keywords: has-patch
Focuses: Cc:

Description

In 3.6+, all formats are supported regardless of defined theme-support, which renders Twenty Thirteen's post-formats support call moot. Patch attached.

Attachments (4)

24109.diff (874 bytes) - added by DrewAPicture 2 years ago.
24109.1.diff (1.3 KB) - added by lancewillett 2 years ago.
24109.2.diff (1.3 KB) - added by DrewAPicture 2 years ago.
24109.3.diff (972 bytes) - added by lancewillett 2 years ago.

Download all attachments as: .zip

Change History (36)

@DrewAPicture2 years ago

comment:1 @kovshenin2 years ago

  • Cc kovshenin added

comment:2 follow-up: @MikeHansenMe2 years ago

  • Cc mdhansen@… added

The Codex will need to be updated as well.

comment:3 in reply to: ↑ 2 @DrewAPicture2 years ago

Replying to MikeHansenMe:

The Codex will need to be updated as well.

We (the docs team) are rolling out a complete refresh of the Post Formats Codex article as part of the 3.6 sprint. We'll have it ready in time to coincide with the release.

comment:4 follow-up: @lancewillett2 years ago

  • Milestone changed from Awaiting Review to 3.6

Interesting. So you're saying only leave the structured-post-formats call in place, for the formats that the theme decides to implement differently than the core behavior?

comment:5 in reply to: ↑ 4 @DrewAPicture2 years ago

Replying to lancewillett:

Interesting. So you're saying only leave the structured-post-formats call in place, for the formats that the theme decides to implement differently than the core behavior?

Yes, exactly. If this were a theme that needed to be backward compatible, we'd need to leave the post-formats call in place. But it doesn't :)

Last edited 2 years ago by lancewillett (previous) (diff)

@lancewillett2 years ago

comment:6 @lancewillett2 years ago

.1 patch removes Video also, leaving just Link that Twenty Thirteen needs to declare as structured. Also adds a bit more explanation in the related PHP comment block.

@DrewAPicture2 years ago

comment:7 @DrewAPicture2 years ago

.2 patch full-stops the formats list.

I also cleaned up the second paragraph for clarity, and added a reference to twentythirteen_get_link_url().

comment:8 follow-up: @kovshenin2 years ago

This makes me think (out loud): do we really have to declare support for structured-post-formats at all? If all post formats are supported by default, isn't it just a matter of using get_the_post_format_* + the_remaining_content versus the_content for back-compat?

comment:9 in reply to: ↑ 8 @lancewillett2 years ago

Replying to kovshenin:

This makes me think (out loud): do we really have to declare support for structured-post-formats at all? If all post formats are supported by default, isn't it just a matter of using get_the_post_format_* + the_remaining_content versus the_content for back-compat?

Ummmmmm. :) That's what we're doing here.

Link does need the declaration since it does not use those new functions.

comment:10 follow-up: @nacin2 years ago

Why does twentythirteen_get_link_url() exist?

The declaration is important for when you *do* use the new functions.

comment:11 in reply to: ↑ 10 @lancewillett2 years ago

Replying to nacin:

Why does twentythirteen_get_link_url() exist?

We use the post permalink as fallback if no link is provided. Something core does not do.

The declaration is important for when you *do* use the new functions.

Just the opposite -- declaring structured means post formats back compat exits early and lets the theme do its own output.

Last edited 2 years ago by lancewillett (previous) (diff)

comment:12 @lancewillett2 years ago

  • Milestone changed from 3.6 to Awaiting Review

Discussing this at length in #wordpress-dev IRC channel today.

Putting this change on hold until the purpose of "structured-post-formats" is clearly delineated.

nacin: Let's not remove anything related to structured-post-formats until core figures out what is going on there.

comment:13 @lancewillett2 years ago

  • Keywords dev-feedback added
  • Milestone changed from Awaiting Review to 3.6
  • Summary changed from Twenty Thirteen: Remove unneeded 'post-formats' support to Twenty Thirteen: Remove unneeded post formats `add_theme_support()` declaration

comment:14 @lancewillett2 years ago

Update: still waiting on core leads for 3.6 + Post Formats team to decide on exactly what the structured post formats declaration is for, and if Twenty Thirteen needs it or not.

comment:15 @lancewillett2 years ago

Discussion during today's dev chat, log: no resolution, still up in the air how a theme should declare if it supports post formats -- and if it needs to additionally declare it's aware of new special content splitting functionality.

comment:16 follow-up: @lancewillett2 years ago

As a side note, and in case it's a possible core bug -- I had to remove the structured call completely on WordPress.com today. It was hiding all the formats but those, Link and Video.

This is how it looks for now:

add_theme_support( 'post-formats', array(
  'aside', 'audio', 'chat', 'gallery', 'image', 'link', 'quote', 'status', 'video'
) );

Which means users there can still see all the radio buttons in the old UI.

comment:17 in reply to: ↑ 16 @SergeyBiryukov2 years ago

Replying to lancewillett:

I had to remove the structured call completely on WordPress.com today. It was hiding all the formats but those, Link and Video.

structured-post-formats declaration overrides post-formats. See the commit message in [23467] and the comment for post-formats in add_theme_support(). Also mentioned in #24135.

comment:18 follow-up: @lancewillett2 years ago

Thinking more about this as I see it in action on WordPress.com and as I look at how we are implementing the vision for structured post formats.

1. Recommendation: change the 'post_formats_compat' filter to to be dependent on themes declaring add_theme_support( 'structured-post-formats' );

add_filter( 'the_content', 'post_formats_compat', 7 );

This now runs for all themes, even for themes that declare post formats the "normal" way (not the new structured way).

Instead, we should make it conditional on specifically opting in to structured post formats so that older themes with existing post formats support will work exactly as they do now. Opt-in instead of forcing them to get the new markup, like Quotes getting blockquote added now—without being intended.

Already getting reports from theme authors where the new markup is breaking their existing styling, for Quotes, for example.

The opt-in action would be simply adding add_theme_support( 'structured-post-formats' ); without arguments.

I think it should work without any arguments, just to turn on the new post content filtering.

Then, if any arguments are passed to the "structured-post-formats" — the theme will be telling WordPress that it does not want the "post_formats_compat" function to kick in; instead it wants to build the output itself.

2. Rename post_formats_compat to post_formats_output or post_formats_content.

This function doesn't really provide compatibility, after all. What it's actually doing is creating default core HTML output for post formats. This is something we should encourage themes to use, because it's based on best practices and clean and semantic HTML5.

By naming it "compat" as we do now, it seems like a fallback rather than the default. Something you don't want to have run ...

If a theme wants to avoid the default HTML output for a particular post format, it can declare add_theme_support( 'structured-post-formats' ); and pass it an array of arguments — the post format by name. Naming the post formats that they wish will bypass the core output, and it's up to the theme to parse the post meta directly or filter the post content itself.

Open questions:

  1. What if a theme uses the new get_post_format_*() functions without any kind of post format add_theme_support declarations?
  2. What if a theme uses the new get_post_format_*() functions without just the non-structured post format add_theme_support declarations?

comment:19 follow-up: @lancewillett2 years ago

Example of a theme that broke: Avid, by The Theme Foundry. It got <blockquote> wrappers around Quote content without the theme's intending it.

http://premium-themes.forums.wordpress.com/topic/why-has-the-quote-post-format-style-changed-to-italics?replies=1

The new get_content_quote() in post_formats_compat() started running for this older theme when 3.6 was merged to WP.com -- and the theme author should probably have control to turn that on. Versus having to opt-out.

comment:20 follow-up: @lancewillett2 years ago

Nacin noted in IRC that core "has no choice" but to parse and output the new structured content, based on user data entry in the UI.

I wanted to note that we don't have the new UI turned on yet on WP.com, so I don't think that's a valid argument for the case with Avid.

And, Quotes are parsed directly from post content -- it doesn't use post meta, right?

comment:21 in reply to: ↑ 20 @DrewAPicture2 years ago

Replying to lancewillett:

And, Quotes are parsed directly from post content -- it doesn't use post meta, right?

Yes, though there is some related quote meta like the source I believe. See #24009

comment:22 @lancewillett2 years ago

I checked the user's exact post on WordPress.com and the meta fields were empty, only content was in normal post content.

comment:23 @tollmanz2 years ago

  • Cc tollmanz@… added

comment:24 in reply to: ↑ 18 @SergeyBiryukov2 years ago

For reference:

  • #19570 — the original concept of admin UI and theme fallbacks.
  • #23347 — the current implementation of theme fallbacks, added in [23450].
  • ticket:23347:25 — the initial structured-post-formats concept, added in [23467], [23468].
  • [23819], [23836], [23899] — the new template functions that appear to have affected the structured-post-formats concept.

comment:25 @philiparthurmoore2 years ago

  • Cc philip@… added

comment:27 in reply to: ↑ 19 @philiparthurmoore2 years ago

Replying to lancewillett:

Example of a theme that broke: Avid, by The Theme Foundry. It got <blockquote> wrappers around Quote content without the theme's intending it.

http://premium-themes.forums.wordpress.com/topic/why-has-the-quote-post-format-style-changed-to-italics?replies=1

The new get_content_quote() in post_formats_compat() started running for this older theme when 3.6 was merged to WP.com -- and the theme author should probably have control to turn that on. Versus having to opt-out.

Noting that the theme in that thread is actually Elemin, another theme affected by <blockquote> wrappers around Quote content without the theme's intending it.

This is the Avid thread: http://premium-themes.forums.wordpress.com/topic/quote-posts-show-2-quotes?replies=10.

IMO themes should need to opt in to using the new structured post formats. The thought of suddenly needing to fix a few hundred old themes doesn't feel very nice; it'd be comforting know that old themes will "just work" as they always have, with or without this new functionality.

comment:28 @lancewillett2 years ago

I'm leaning toward just the normal list with post-formats support for Twenty Thirteen. We don't need to declare the structured at all for any of the post formats -- Link compat markup works just fine.

@lancewillett2 years ago

comment:30 @lancewillett2 years ago

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

In 24151:

Twenty Thirteen: remove unneeded "structured" post formats add_theme_support() declaration. Fixes #24109.

comment:31 in reply to: ↑ 29 ; follow-up: @SergeyBiryukov2 years ago

Replying to lancewillett:

current_theme_supports( 'post-formats' ) is still in use in several places

See #24219 for Press This and XML-RPC.

comment:32 in reply to: ↑ 31 @DrewAPicture2 years ago

Replying to SergeyBiryukov:

Replying to lancewillett:

current_theme_supports( 'post-formats' ) is still in use in several places

See #24219 for Press This and XML-RPC.

As long as structured-post-formats support is added, adding post-formats support is redundant. Currently works fine with Twenty Thirteen in Press This, not sure about XML-RPC.

Note: See TracTickets for help on using tickets.