Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OwnYourGram posts not pulling in photos #51

Closed
davidmead opened this issue May 13, 2017 · 35 comments
Closed

OwnYourGram posts not pulling in photos #51

davidmead opened this issue May 13, 2017 · 35 comments

Comments

@davidmead
Copy link

Since updating my syndicated posts from Instagram, via OwnYourGram, are not including the photo when being being created.

What I'm getting is the WP post being created with all the content pulled across, but, the image is in the media library and the normal code isn't being generated to show it.

Post with the old version of SemPress...

<div class="e-content">
First fill of my @growlerwerks uKeg is @goldhornbrew's EVIL SPIRITS SCHWARZBIER! &#x1f37b;
#vscocam #drink #beer #instagram
</div>
[gallery size=full columns=1]

Post with the new SemPress

Beer o'clock &#x1f37b; @jdramirez2
'Evil Spirits' from @goldhornbrew
#drink #vscocam #instagram

As you can see the <div> and [gallery] aren't being created.

Any ideas?

@pfefferle
Copy link
Owner

I don't think this is a theme problem, because there is no functionality to filter the post output and there were no changes since the last version. was there an update of one of the other plugins you use?

do you use micropub and ownyourgram?

@davidmead
Copy link
Author

davidmead commented May 13, 2017

I do use micropub and ownyougram. Though the micropub plugin update came out a month ago and I haven't had any issues.

Wondering if some code I'd added was blown away. I'll start digging there though.

@pfefferle
Copy link
Owner

I am not aware of any change that might cause this issue. Can you check if the html is in the DB? SemPress might only be filtering the output and have no impact of saving new data. So if it is correctly saved to the db, it might be a theme issue, if not, there might be another plugin.

@davidmead
Copy link
Author

davidmead commented May 15, 2017

I rolled back to version 1.4 of SemPress and posted a photo to Instagram, PESO'd to my blog with OwnYourGram and it is now working as before.

Even posting the same code block and pulling the image in

<div class="e-content">
Steeple?
#vscocam #instagram
</div>
[gallery size=full columns=1]

I didn't change any other plugins.

I'm really not sure what is wrong.

@pfefferle
Copy link
Owner

Can you check if it might be the same problem as #52 ?

@chrisaldrich
Copy link

@davidmead, Not directly related, but in lieu of OwnYourGram, which is very solid, you might also want to take a look at https://github.com/jtsternberg/DsgnWrks-Instagram-Importer which is a WordPress specific Instagram importer. Because it was built specifically for WordPress it's got a lot of additional pre-built functionality that allows you to set Post Format, Post Kind (if you're using Post Kinds plugin), handle hashtags, Import your entire Instagram history (not just from when you started using OYG), set the photo as Featured (or not), categories and tags, and a bunch of other subtle bells and whistles that are WordPress specific and not well supported by OYG. Otherwise, it works much like OYG does on the back end and includes all of the meta-data including GPS coordinates and location data if you choose to include it in your post.

@davidmead
Copy link
Author

OK @pfefferle.

I updated every plugin and theme so everything is up-to-date.

Just posted to Instagam, went into OwnYourGram and clicked post there so I didn't have to wait, and what I have in the db is as follows...

ID = 16601
post_content = test#instagram
post_date = 2017-05-21 10:50:08

No image in the HTML. Above that is ID = 16602 which is the image for that post, which seems like the way it's always been done.

Last Instagram image, before all the updates, was ID = 16402 and had the following

post_content = <div class="e-content">Ice cream on a rainy night.#instagram</div>[gallery size=full columns=1]

Really in the blind here.

@davidmead
Copy link
Author

Thanks for the recommendation @chrisaldrich. At this point I'm feeling like throwing the towel in on syndication as a whole :-(

@pfefferle
Copy link
Owner

I am very sorry, never thought that I changes something that fundamental. I will have a look at ownyourgram to hopefully find the problem!

@pfefferle
Copy link
Owner

OK, I can reproduce the problem! that is really strange! 'will have a look what exactly happens here, later today.

@pfefferle
Copy link
Owner

@dshanske @snarfed there seems to be a problem with SemPress, ownyourgram and the micropub plugin. Do you have an idea, why the image might be ignored when there is also a caption? if there is only an instagram picture without caption/text, everything works as expected.

@snarfed
Copy link

snarfed commented May 22, 2017

sorry for the trouble, all! i suspect it's this: https://github.com/snarfed/wordpress-micropub/blob/4eb0e217685a653957eb4d9604bd848f25ec41f5/micropub.php#L516-L524

micropub generates and injects HTML into post content for things like replies, reposts, likes, rsvps, events, and photos. it doesn't do that if post kinds is installed or the current theme support mf2, though, since it expects them to do it.

i think we've settled on a standard way across plugins to store structured mf2 in wordpress - post metadata with mf2_ prefix, all arrays - but i'm not sure we consistently agree on which plugins/themes are responsible for rendering them to HTML, or when.

what do we want to render mf2_ post metadata? plugin(s)? theme? both? and when? at create/edit time, into post_content, or at page render time?

in this case, it sounds like sempress doesn't render photos, but micropub thinks it does, at write time, which is why micropub notices that post content is empty when there's no caption and renders the photos as a fallback.

@pfefferle
Copy link
Owner

oh ok... that's it!

@pfefferle
Copy link
Owner

@snarfed you are right, SemPress does not render images, but it says that it supports mf2 https://github.com/pfefferle/SemPress/blob/master/sempress/functions.php#L102

@pfefferle
Copy link
Owner

pfefferle commented May 22, 2017

@snarfed I think we should add another theme support key. supporting mf2 markup does not imply to render stuff saved in mf2_* meta fields.

what do you think about: add_theme_support( 'microformats2-rendering', array( 'mf2_photo', 'mf2_name' ) );

@snarfed
Copy link

snarfed commented May 22, 2017

i like it! especially if the values are the exact mf2_ post meta keys that are supported. let's do it!

i wonder if post kinds could even do something similar, to be explicit about which mf2 properties it can and can't render. @dshanske? is the some shared way that both plugins and themes can declare their support for individual things like this?

@pfefferle
Copy link
Owner

pfefferle commented May 22, 2017

cool! yes, if they exactly match the mf2_ meta field you are able to filter very granular!

for plugins we could add a filter like:

function custom_plugin_disable_microbub_rendering( $for ) {
    $for[] = 'mf2_photo';

    return $for;
}
apply_filters( 'microbub_disable_rendering', 'custom_plugin_disable_microbub_rendering' );

@pfefferle
Copy link
Owner

@davidmead long story short: we have to change something in the micropub plugin and everything should work as expected again.

@snarfed
Copy link

snarfed commented May 22, 2017

hmm. if sempress and post kinds both support rendering some property, how do they decide which one actually should?

other WordPress plugins must have had this problem before with common functionality. do you know what they do? filters with priorities? something else?

@pfefferle
Copy link
Owner

hmm... that is another good question...

@pfefferle
Copy link
Owner

theme should always be first, so the plugin could also check the theme support... but what if two plugins can render the same property....

@snarfed
Copy link

snarfed commented May 22, 2017

another problem: micropub renders at write time, into post_content, but sempress and post kinds (i think) render to HTML at page load time.

so, when a post is created, if post kinds doesn't support a property but micropub does, micropub will generate HTML for it into post_content. if post kinds (or sempress) later adds support for that property, it will show up on the page twice, since it's already in post_content.

thoughts? i suspect this means micropub should stop generating its own HTML into post_content. :/

sorry for the brain dump. feel free to grab me on IRC instead if you want!

@dshanske
Copy link

This has been a problem that Core has discussed also in their new content block explorations, and in the Post Formats UI that was explored in (I think) 3.6. How do you generate dynamically and support statically?

https://make.wordpress.org/core/2017/01/17/editor-technical-overview/

This is where WordPress seems to be going with that idea. It seems like if this work comes into core, we might be able to enhance it to address the issues in the long term.

With Post Kinds, the advantage of storing and generating is that the rendering can change and improve over time. I intend to add more support for rendering properties. In the the last few versions, Post Kinds actually supports the theme taking over rendering.

The microformats2 support suggests the theme supports it...which would reflect the structure of the theme itself.

I wonder if there is a way to combine the Micropub generation with Post Kinds to form something different? I have no problem if we separate the rendering from the UI.

@davidmead
Copy link
Author

@snarfed @pfefferle Thanks for digging into this.

I always seem to be the one "breaking" #indieweb stuff on Wordpress ;-)

Look forward to updating the plugins/themes soon.

@pfefferle
Copy link
Owner

@davidmead if you need a quick fix, just remove this line https://github.com/pfefferle/SemPress/blob/master/sempress/functions.php#L102

@snarfed
Copy link

snarfed commented May 23, 2017

@dshanske et al:

I wonder if there is a way to combine the Micropub generation with Post Kinds to form something different? I have no problem if we separate the rendering from the UI.

sure! to start, i should change micropub's rendering code from injecting into post_content at write time to rendering at page load time.

then, we need a way for themes and plugins to cooperatively determine who renders which mf2 property. here's a straw man proposal: themes and plugins hook into mf2_[PROPERTY]_renderer filters for each property they can render, at a priority they choose. the filter returns the name of the theme/plugin that should render the property. after the highest priority theme/plugin takes ownership by returning its name, all lower priority filters simply pass that name through.

function my_plugin_mf2_renderer( $renderer ) {
    return $renderer ?: 'my_plugin';
}
add_filter( 'mf2_pronoun_renderer', my_plugin_mf2_renderer, 2, 1 );
add_filter( 'mf2_photo_renderer', my_plugin_mf2_renderer, 99, 1 );

when a theme/plugin is ready to render an mf2 property, it first calls the filter. if it gets its name, it renders the property. otherwise, another theme/plugin has higher priority for this mf2 property, so the current plugin skips rendering it.

if ( apply_filters( 'mf2_pronoun_renderer', NULL ) == 'my_plugin' ) ) {
   echo( implode( '/', $mf2_pronouns ) );
}

it's similar to *_theme_support[s] and *_post_type_supports, but allows multiple plugins/themes to gracefully take or relinquish ownership. thoughts?

@dshanske
Copy link

You can actually do a more dynamic filter using apply_filters( "mf2_{$type}_renderer",$type...

There is also the option of registering the configuration to a global variable, which is how post types and other objects are done.

@dshanske
Copy link

Would this be a possible extension of wordpress-uf2? Turning it into a mf2 rendering library that can be installed standalone or incorporated into other plugins?

@dshanske
Copy link

@snarfed There is one other alternative. Post Kinds uses a template system where a template piece is used for the specific post type presentation. It can be overridden in the theme. Wonder if that is worth expanding adoption on.

@pfefferle
Copy link
Owner

pfefferle commented May 23, 2017

I think I would start with:

  1. change the direct content creation into a late template replacement/filter.
  2. don't add any mf2 markup at all (<div class="e-content"> or <div class="p-name"> for example) except for elements like images or contact data
  3. the microformats2 theme support should only be interpreted by plugins like uf2 and perhaps Post Kinds (I am not aware of the functionality atm.)
  4. implement some kind of "mf2_pronoun_renderer"

what do you think?

@pfefferle
Copy link
Owner

I like the idea of a generic filter system that can be used by various plugins/themes, btw.

@dshanske
Copy link

Post Kinds is three pieces. A taxonomy to classify the post types for archives and such. A post UI enhancement that adds fields and will parse URLs for link preview data. A mf2 renderer that adds mf2 properties to content which is themeable.

That is why I am saying that I am willing to spin the rendering stuff out of Post Kinds, merge it with the rendering part from Micropub so that we aren't having two things do the same function. Then Post Kinds becomes the UI enhancements. Not sure what you think re the Taxonomy.

@snarfed
Copy link

snarfed commented May 23, 2017

thanks for the feedback, guys!

@pfefferle, i think i mostly understand. i'll put together a PR for micropub and cc you on it. feel free to do the same for sempress! no hurry.

@dshanske maybe! i'd like to avoid scope creep here though. regardless of where any functionality lives, we'll always have cases where more than one plugin/theme can render a given mf2 property,
so they'll need to decide who actually should. i'm inclined to tackle just that here. we can discuss moving existing functionality too, but let's maybe keep that separate for now.

@dshanske
Copy link

I will be happy to implement the proposed filters in Post Kinds for the time being as well. Suggest the variable version I suggested though

@pfefferle
Copy link
Owner

I will close this, as it is not a SemPress problem...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants