#23347 closed enhancement (fixed)
Theme fallbacks for post format meta data
Reported by: | helen | Owned by: | |
---|---|---|---|
Milestone: | 3.6 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Post Formats | Keywords: | has-patch commit |
Focuses: | Cc: |
Description
If a theme doesn't declare support for a format, we should have output fallbacks available for hooking onto the_content using nice semantic HTML.
IRC reference: https://irclogs.wordpress.org/chanlog.php?channel=wordpress-dev&day=2013-01-31&sort=asc#m544599
Attachments (14)
Change History (77)
#4
@
12 years ago
- Type changed from defect (bug) to enhancement
Are you working on this Beau? If not I'd like to volunteer to knock up the first version in plugin form along with a WXR.
#5
follow-up:
↓ 6
@
12 years ago
The markup for the image format isn't quite correct.
<div class="wp-caption format-image-img"> <img src="{$_format_url}" alt="{$post_title}" height="" width="" border="0" /> <div class="wp-caption-text">{$post_content}</div> </div>
should be:
<div class="wp-caption format-image-img"> <a href="{$_format_url}"> <img src="{$featured_image_url}" alt="{$post_title}" height="" width="" border="0" /> </a> <div class="wp-caption-text">{$post_content}</div> </div>
with a bit of logic to only add the link if a URL has been entered.
#6
in reply to:
↑ 5
@
12 years ago
Replying to johnbillion:
The markup for the image format isn't quite correct.
...
Thanks for the (accurate) correction johnbillion; definitely need that <a> in there.
I haven't started on a plugin or test data, so if you'd like to handle those bits, that'd be excellent. LMK if you'd like a hand with any of it.
#7
@
12 years ago
I realized I missed a field in the Quote format, with all the mess around it :)
In the latest mocks, there are the following fields:
- Quote (the actual quote)
- Who said (source name)
- Where did they say this? (source URL? I think?)
- Comment (my commentary on the quote)
If we formalize the "Where did they say this" to be "URL to source" and only allow a URL (which is what I think we should do), then we can use that directly in the cite="" attribute on the <blockquote>.
<div class="format-quote-quotation"> <blockquote cite="{$_format_url}">{$post_content}</blockquote> <div class="format-quote-source"><a href="{$_format_url}">{$_format_source}</a></div> </div> . "\n\n" {$_format_commentary}
(change is $url => $_format_url, to use the standardized meta field, and wrap the _format_source in a link to _format_url, since the cite attribute isn't actually exposed in most (any?) user agents)
If the "Who said" field is indeed the name of a person, then HTML5 dictates we cannot use the <cite> tag around it. If it was the name of a "text" (document, script, some body of work) then we could wrap it in a <cite>. Annoyingly, pre-HTML5 it would have been fine to wrap whatever was entered in this field with <cite>, which would have given nice clean markup.
#8
@
12 years ago
For quote source URL, let's assume it's a shared URL field with things like the link format, and I think image as well. Not sure what to think about the semantics of the markup. Dangit, cite.
For the image, do we imagine that the data could be an attachment ID as well as a URL? I was sort of thinking it could, but not sure. Audio and video are similarly flexible inputs - could be an oEmbed URL, a shortcode (hoping for a bundled player), or copy-pasted HTML embed markup. I don't think it really makes much of a different in practice for audio/video compat output, but thought I'd note it.
For the chat format, I'm not convinced/sold on doing anything to it markup-wise at all.
#10
follow-up:
↓ 13
@
12 years ago
Linked on Make/Core about the quote format: http://synapticism.com/working-with-wordpress-post-formats-and-quotations/
For the record, I'm just a personal crankmonster about the chat format. If it's just splitting up lines and applying some markup that can be styled, then that's just fine. :) Would say that it might not be good to use default markup that could potentially look extremely different from paragraphs, though. Maybe in that way we need to think about how a theme could define what the marked-up output is.
#13
in reply to:
↑ 10
@
12 years ago
Replying to helen:
Linked on Make/Core about the quote format: http://synapticism.com/working-with-wordpress-post-formats-and-quotations/
While I like this approach for HTML5 (minus the date, personally I think that's overkill); unfortunately it doesn't translate too well to <HTML5. We could maybe just take the general structure of it and instead of the <footer> and <article>, use <div>s still.
#14
@
12 years ago
- Keywords has-patch added
Attached is an initial run - can be tweaked obvs but lays the groundwork. Audio / Video still have hard dependency on #23282 which works, still needs tires kicked.
In my attached patch, I did not go crazy with wrapper divs a la Beau, they can be added at will, as can more filters. Everything works great in 2011 theme, minus the status which adds 65px margin-bottom to the entry content, and the audio / video shortcodes just get output on the page raw.
#15
@
12 years ago
Quick testing notes:
- Image compat = awesome!
- Audio/video media field compat doesn't do oEmbed. Could also use detection to make sure whatever's provided is not already in the content.
- Chat has some tricky things - what if the person uses paragraphs (two new lines) instead of line breaks (single new line) to separate lines of the chat? We lose the paragraphs here and get <span><br /></span> kind of stuff. The break tags shouldn't be inside the spans in any case. Also, even/odd classes are probably a good idea here.
- Quote puts URL as the source, probably just a sprintf accident. Also could use de-dupe detection.
#17
@
12 years ago
attached new patch, fixed(?) oEmbed by wrapping URL in [embed]
Introduces, get_post_format_meta()
which will return an associative array of "key minus the _wp_format_ part" => $value
#18
@
12 years ago
23347.3 makes a few modifications/additions to .2:
- Tries to detect "duplicate" content; where the user has already entered the format-specific data within the_content, but has also filled out the specific fields. In this case we always trust the user to have entered what they want to appear in the_content
- Adds a check after the main switch in
post_formats_compat
so that if no$format_content
has been created, then we return$content
without modifying it (adding tags/classes); basically only modifying things when we explicitly have something to offer - Uses
stristr
rather thanpreg_match
for the image check, since if the URL is there at all, let's just assume the user knows what they're doing - Beefs up the gallery detection, which was previously only checking the first shortcode encountered. Now checks all shortcodes and bails if any of them are a gallery.
get_post_format_meta()
has been moved to #19570, so I'm leaving it out of this diff.
@
12 years ago
Removed the chat fallback entirely, until we can decide what to do with it. Also bailed earlier in the function for aside/status formats.
#19
@
12 years ago
23347.5.diff continues on from .4:
- Made the compat wrapper class generic - thinking CSS should look like
.chat .post-format-content
rather than.chat-post-format-content
.post_class()
gives that to us, and a filter is included should a theme author really want to do something to it. - Compat wrapper is only wrapped around the compat output itself, not around the entirety of the content + compat output. There was some funkiness happening because of that, and it doesn't make much sense the other way around.
- Removed audio/video shortcodes for now, because it's useless without #23282. If we get #23282 done, we can restore it.
- As noted for .4, chat is removed because it just wasn't working well and we want to get this in soon. Iteration welcome :)
Thing I noticed, not sure if we need to address now or not: using the embed shortcode in the fallback for audio/video means that if the user enters something funky like "http://blah.com and some more stuff" it esc_url()
s the whole thing and makes it a link in WP_Embed::maybe_make_link()
. It happens if you do it in the content too, so maybe it's something to make better in maybe_make_link()
, or it's fine because that would be a weird thing to do, anyway.
#20
@
12 years ago
$compat['class']
needs esc_attr()
, and $compat['tag']
should probably do something similar (maybe even a whitelist of tag names?)
Other than that, this looks ready for a first pass commit to me.
#23
@
12 years ago
Pulling this conversation from #19570 since it belongs in this ticket.
Replying to helen:
Replying to greenshady:
Support for post formats for several versions has meant something different. It simply meant that a theme supports post formats in some way. Now, this meaning is being changed to say that a theme handles specific metadata too.
I don't know if it's "too" so much as "instead". The UI for all formats shows now no matter theme support, rather than having the theme determine the UI (all of some radio buttons before). This is in line with our goal of providing something that users can rely on to be consistent. I don't know how we can move forward unless we make at least some kind of break from the previous paradigm. Hoping that theme compat can address as many situations as possible to build trust with users and yet not disrupt everything for theme authors. Again, relying on those who build themes regularly, especially public release ones, to really help us shape this piece.
The theme compat and UI stuff are great. That's not the big issue. We need a solution for existing themes that currently expect post format data to be stored in $post->post_content
. That's where it's always been. So, we need solutions for:
- Themes that don't support post formats.
- Themes that support post formats and the new metadata.
- Themes that support post formats and aren't using the new metadata.
The work thus far has focused on the first two items. We need to also focus on the third.
My proposal is that we make the theme support call look something like the following.
add_theme_support( 'post-formats', array( 'aside', 'gallery' ), array( 'compat' => false ) );
The compat
argument:
- If
true
(default), would add the theme compat layer. Basically, it adds everything where it's expected (the post content). - If
false
, would remove the theme compat layer. This would allow themes to explicitly say that they will use the new metadata and don't need WP to handle it.
This will be a good solution for two reasons:
- Users will be able to use the new post format data with existing themes that support post formats.
- New themes can simply allow WP to handle how the data is output if they choose this route.
#25
@
12 years ago
The Different Situations
Three theme situations:
- Theme doesn't support post formats
- Theme supports old-style post formats
- Theme supports new-style "structured" post formats
A — Three fallback situations:
- Theme does not support a format, but the post has new-style "structured" post format data
- Fallback: Turn new-style structured data into HTML and use the_content filter
- Theme supports a format (old-style), but the post has new-style "structured" post format data
- Fallback: Turn new-style structured data into HTML and use the_content filter
- Theme supports a format (new-style), but the post has old-style unstructured post content
- Fallback: Parse out post_content into individual pieces of post format data. (This needs to occur, because this theme wouldn't necessarily have a call to the_content() for all post formats, would it?)
B — Three situations where do not 'fall back':
- Theme does not support a post format, and the post has old-style unstructured post content
- The theme supports a format (old-style), and the post has old-style unstructured post content
- Theme supports a format (new-style), and the post has new-style post format data
To reorganize these content situations based on the individual theme situations: (someone can turn this into a flowchart if they'd like)
- Theme doesn't support a post format:
- The post has new-style "structured" post format data. Fallback: Turn it into HTML, use the_content.
- The post has old-style post content: Do nothing.
- Theme supports a post format, old-style:
- The post has new-style post format data: Fallback: Turn it into HTML, use the_content.
- The post has old-style post content: Do nothing.
- The theme supports a post format, new-style:
- The post has new-style post-format data: Do nothing.
- The post has old-style post content: Fallback: Parse post_content into pieces of post format data.
Old versus New Formats
It is clear here that a theme not supporting a format is pretty much the same as supporting old-style formats. This is even more apparently now that we will be allowing a user to use any formats they wish, even if the theme does not support it, so the old-style "support" becomes pretty close to meaningless.
Also, since each format is individually supported or not supported, you could have many of these situations occurring on the same site, without a teme switch:
- A theme supports the quote format, with new-style structured data (B3)
- The site also has chat posts written with metadata (A1 — must convert these into HTML)
- The site has link formats without metadata (B1 — show these as-is)
- It also has old-style quotes (A3 — must parse these into pieces)
Or:
- A theme supports the quote format, old-style (B2)
- The site has quote formats written with metadata (A2 — must convert these into HTML)
- The site has chat posts written with metadata (A1 — must convert these into HTML)
- The site has chat posts written without metadata (B1)
Theme Support
It is probably apparent by now that we need to know when a theme supports a new-style format in particular. Whether the theme supports an old-style format in only has to do with whether the theme has a particular design for that format, and isn't very helpful anymore.
To make this change, I suggest we come up with a new theme support argument. Specifically, structured-post-formats:
add_theme_support( 'structured-post-formats', array( 'quote' ) );
This would replace post-formats support. It is a trivial change, but would be necessary because themes would have to make bigger changes anyway — specifically, switch to new template tags to support the new-style metadata.
Under the hood, we could automatically register support for post-formats when structured-post-formats is registered. Then you can have checks like current_theme_supports( 'post-formats' ) && ! current_theme_supports( 'structured-post-formats' )
to see if a theme supports post formats generally, but not the newer structured ones. (Note that I would prevent in code the ability for a theme to say "Yes I support these formats" with some being old-style, others being new-style. As said above, there is no purpose for this — in terms of working with content, supporting old-style is the same as not having the format supported at all. So it is best to avoid such an API rift.)
Compatibility Across WordPress Versions
If a theme wishes to remain compatible with 3.5 while still supporting 3.6 methods, they'll need to do a few things. (I would recommend doing this for Twenty Ten and Twenty Eleven, and leaving Twenty Twelve for 3.6+ only, to set different examples and to keep 3.6 clean. Supporting forward for an older theme is important; supporting backward for a newer theme is not.)
- They'd need to call add_theme_support() for both post-formats and structured-post-formats. The former would get ignored in 3.6, the latter would get ignored in 3.5.
- They'd need to function_exists( 'get_post_format_meta' ) before using the new-style functions. In their
else
, they'd use the_content() as before.
Feeds
One final note, everything goes out the window when dealing with an RSS feed. Everything needs to be piped back into the_content(). is_feed()
does not appear to be accounted for in [23450].
#26
@
12 years ago
Noticed a couple of PHP_EOL
references in [23450]:
http://core.trac.wordpress.org/browser/trunk/wp-includes/formatting.php?rev=23450#L1959
We don't use that constant anywhere else in core. Could it be just "\n"
for better readability (23347.newline.diff)?
#27
@
12 years ago
Related #23570 - the functions introduced there can exist without any knowledge of fallbacks
#32
@
12 years ago
The new post_formats_compat function seems to be doing something stupid. It was automatically adding a [gallery] to the end of posts with the gallery post format for me, even though those posts already contained a gallery (after a !--more-- tag).
Note that the theme in question does declare support for the gallery format like so:
add_theme_support( 'post-formats', array( 'aside', 'audio', 'chat', 'gallery', 'image', 'link', 'quote', 'status', 'video' ) );
I think that post_formats_compat needs to be done away with, or altered, or somehow modified to not be changing existing content already being produced by sites and themes.
This code currently breaks existing sites in major ways, and should be considered a blocker.
#33
@
12 years ago
It's supposed to be smart about not duplicating things that are found in the existing content - I imagine there's something going wrong that needs fixing. I don't know what's "blocking" about it, as this is definitely work in progress. There's a whole other side we're still tackling (new theme, old data). See http://make.wordpress.org/core/2013/02/22/post-formats-ui-update-221/
#34
@
12 years ago
It's a blocker in that it breaks existing content on existing sites. Changing pre-existing site content, in any way, is a regression and thus a blocker.
Fallback output should be eliminated entirely. IMO. You can make it smarter all you like, but the moment somebody's actual site content changes from an upgrade, then the upgrade is instantly "broken", capable of "breaking people's sites", and roundly hailed as yet-another-failure of the WordPress development team.
I would suggest reverting this entirely and marking as wontfix, unless you can correct it such that it does not change displayed content on existing sites. If you want to handle new-sites with old-themes, that's fine, but you can't just flat out break pre-existing sites willy-nilly like this.
#35
@
12 years ago
Note, this can be as simple as having a flag somewhere to not add the post_formats_compat filter function to the content on upgraded sites, thus only allowing it to apply to new sites.
#36
@
12 years ago
(Otto is mostly right,) it's because the full picture of how this is gonna work is not in core yet, and there has been no iteration really since the initial commit, and not against a defined set of test cases. Let's discuss the battery of related tickets in dev chat today and a plan for not breaking anything
#37
@
12 years ago
Scenarios
- If the post doesn't have a post format or the theme supports structured-post-formats, nothing should happen to content
- If you don't support structured-post-formats and you don't have gallery, audio, or video metadata, we are currently adding the shortcode to
the_content()
- you said "it's audio!", didn't opt in to structured, and have no meta, you are getting a new shortcode when:- you have no audio-like URL, embed, or shortcode in your content,
- but you DID attach audio to your post.
So the main question here: should we add shortcodes for attached media (post_parent is the current post) when nothing in the content or meta is present? Otto says no. I personally hate the idea that media can only have one post_parent as is, so I lean towards no when there is no meta and nothing explicitly looking like gallery|audio|video or a URL in the content.
#38
@
12 years ago
Now, let's open our hymnals to the Codex: http://codex.wordpress.org/Post_Formats
video - A single video. The first
<video />
tag or object/embed in the post content could be considered the video. Alternatively, if the post consists only of a URL, that will be the video URL. May also contain the video as an attachment to the post, if video support is enabled on the blog (like via a plugin).
"May also contain the video as an attachment to the post" - what do we do here? Have people been following these rules? Should they have been? Does it matter?
#39
@
12 years ago
For the case of brand new websites running themes without structured-post-formats but with normal-post-formats, I'm okay with post_formats_compat(), although it does need some minor improvements.
The specific problem I encountered was basically because it's only examining the input $content data, which doesn't get the complete content in the case of a !--more-- tag situation (and probably will have the same issue with !--nextpage--). For it to work properly, post_formats_compat() needs to examine the entire post. It can do this by looking at the $post->post_content directly instead and that would fix the immediate issue. Minor change to the preg_match_all on line 371 would probably do the job.
For the case of older websites, I'd rather it not do anything at all, or even have that filter be connected. Many people with existing sites won't be happy to upgrade and notice posts looking different, even if we think they may be "better". Set an option on upgrade to prevent that filter from attaching and it's fine. This isn't without precedent, look at the Links menu in 3.5.
No opinion on audio/video as yet, however it's not unfeasible that some people have used the video format and then been posting something like youtube links in the post and such. Making any assumptions here about what the old-post should contain is probably ill-advised.
#40
@
12 years ago
On an old site with old data, what this currently should do is a whole lot of nothing. Seems like it was the gallery shortcode default that made that a false statement, so removing the default shortcodes from compat (just gallery right now) is probably a good idea. We shouldn't make that kind of assumption about content. Fixing the matching to look against the entirety of the post content is also a good idea, anyway, assuming that's also broken.
#41
@
12 years ago
Okay, I think I see the issue - the compat fallback doesn't take more/excerpted content into account, both in terms of matching and display. It'll put the compat output after the more link by default, which is definitely bad. I think we'll need to output the compat stuff (noting that it should only show with old themes and new data right now) regardless of the presence of more/nextpage by default, but we need to place it better as a default.
#50
@
12 years ago
- Keywords commit removed
Thanks. There's been a LOT of movement here and I fear I'm losing track a bit. What's still needed for this ticket?
#51
@
12 years ago
I think we need to fix what happens with appended content when there's a more tag present, and probably also paging (haven't actually tested the latter). Right now things that are positioned afterward will come after the more link on an archive.
Also, looking at it - seems like the filter on the defaults array gets ignored for some of the values for the link case?
#54
follow-up:
↓ 55
@
12 years ago
- Keywords close added; has-patch removed
Are there still issues here? seems like we *might* be able to use new tickets?
#55
in reply to:
↑ 54
@
12 years ago
Replying to wonderboymusic:
Are there still issues here? seems like we *might* be able to use new tickets?
See my last comment. Fine with opening new tickets, but the second part is a question. :)
#56
follow-up:
↓ 57
@
12 years ago
- Keywords has-patch commit added; close removed
23347.12.diff respects $compat
vals - 2013 will need to filter if they want link to come before content
#57
in reply to:
↑ 56
@
12 years ago
Replying to wonderboymusic:
23347.12.diff respects
$compat
vals - 2013 will need to filter if they want link to come before content
Should be fine, in Twenty Thirteen we use the link URL on the title of the post.
Apologies, it's long :)
Goals/Constraints
Post Formats Data Fields/UI -- note I made up meta key names to demonstrate consistency, those are not final
Post Formats Default HTML Output
Further Discussion