WPBuddy / largo

A WordPress framework for news websites. Finely-crafted by INN and expertly-honed and maintained by the technology team at WP Buddy.
http://largo.wpbuddy.co
GNU General Public License v2.0
170 stars 83 forks source link

Inserting an image with a credit but no caption results in no [caption] shortcode #933

Open benlk opened 9 years ago

benlk commented 9 years ago

From HELPDESK-428:

If a caption is not provided, the caption shortcode does not get inserted (we just get an <img> tag instead) so WP is unable to parse it and show the credit.

benlk commented 9 years ago

The image credit is provided by the Navis Media Credit plugin, which Largo ate and currently lives in lib/navis-media-credit.

The problem is in here: https://github.com/INN/Largo/blob/master/lib/navis-media-credit/navis-media-credit.php#L142-L147

aschweigert commented 9 years ago

this code is also just generally a mess

aschweigert commented 9 years ago

Probably also not a bad idea to nuke all the navis and argo prefixed things and change them to largo_, account for that in the update process, etc.

benlk commented 9 years ago

The argo and navis prefixes aren't breaking things, though, and changing the prefixes should require a lot of testing to make sure that nothing breaks by changing function and filter names. That should probably be another ticket.

aschweigert commented 9 years ago

Yeah, i agree. Just wanted to note it.

benlk commented 9 years ago

Created that issue: https://github.com/INN/Largo/issues/935

benlk commented 9 years ago

Hmm. Removing that check, and fixing a regular expression, results in unclosed <a> tags:

<p>Going to add an image with a caption:</p>

<div id="attachment_10821"  class="wp-caption module image alignnone" style="max-width: 38px;">
    <a href="http://youthtoday.vagrant.dev/files/2015/11/excellent-1417755785@2x.png">
        <img class="size-full wp-image-10821" src="http://youthtoday.vagrant.dev/files/2015/11/excellent-1417755785@2x.png" alt="Ehehehe" width="38" height="60" />
    </a>
    <p class="wp-caption-text">Ehehehe</p>
</div>

<p>And now one with a caption and credit</p>

<div id="attachment_10822"  class="wp-caption module image alignnone" style="max-width: 336px;">
    <a href="http://youthtoday.vagrant.dev/files/2015/11/Ashleigh-soccer-ball.jpg">
        <img class="size-medium wp-image-10822" src="http://youthtoday.vagrant.dev/files/2015/11/Ashleigh-soccer-ball-336x233.jpg" alt="mufufu" width="336" height="233" />
    </a>
    <p class="wp-media-credit">schloink</p>
    <p class="wp-caption-text">mufufu</p>
</div>

<p>And now one with just a credit</p>

<div id="attachment_10823"  class="wp-caption module image alignnone" style="max-width: 336px;">
    <a href="http://youthtoday.vagrant.dev/files/2015/11/tumblr_nx58zywltJ1qes1u6o1_1280.jpg">
        <img src="http://youthtoday.vagrant.dev/files/2015/11/tumblr_nx58zywltJ1qes1u6o1_1280-336x336.jpg" alt="tumblr_nx58zywltJ1qes1u6o1_1280" width="336" height="336" class="size-medium" />
    <p class="wp-media-credit">bwahaha</p>
</div>

<p>But what about just an organization?</p>

<div id="attachment_10824"  class="wp-caption module image alignnone" style="max-width: 336px;">
    <a href="http://youthtoday.vagrant.dev/files/2015/11/unnamed.png">
        <img src="http://youthtoday.vagrant.dev/files/2015/11/unnamed-336x67.png" alt="unnamed" width="336" height="67" class="size-medium" />
    <p class="wp-media-credit">glorp glopr lrpepo</p>
</div>

<p>And nothing at all?</p>

<div id="attachment_10825"  class="wp-caption module image alignnone" style="max-width: 128px;">
    <a href="http://youthtoday.vagrant.dev/files/2015/11/B_r.jpg">
        <img src="http://youthtoday.vagrant.dev/files/2015/11/B_r.jpg" alt="" width="128" height="128" class="size-full" />
</div>

So not having a caption means that the closing </a> doesn't get added. :-1:

benlk commented 9 years ago

The closing </a> would come before the $credit check here: https://github.com/INN/Largo/blob/master/lib/navis-media-credit/navis-media-credit.php#L196

        $out = sprintf(
            '<div %s class="wp-caption module image %s" style="max-width: %spx;">%s',
            $id,
            $align,
            $width,
            do_shortcode( $content )
        );
        if ( $credit ) { 
            $out .= sprintf( '<p class="wp-media-credit">%s</p>', $credit );
        }

So something must be wrong with do_shortcode($content);

benlk commented 9 years ago

Nope, do_shortcode($content) is in this case returning what it's fed:

var_log($content):

<a href="http://youthtoday.vagrant.dev/files/2015/11/B_r.jpg"><img src="http://youthtoday.vagrant.dev/files/2015/11/B_r.jpg" alt="" width="128" height="128" class="size-full" />

var_log(do_shortcode($content)):

<a href="http://youthtoday.vagrant.dev/files/2015/11/B_r.jpg"><img src="http://youthtoday.vagrant.dev/files/2015/11/B_r.jpg" alt="" width="128" height="128" class="size-full" />
benlk commented 9 years ago

$content is passed to caption_shortcode( $text, $atts, $content ) without the closing </a>, and caption_shortcode() is a filter on img_caption_shortcode.

The second-to-last paragraph in the HTML above is saved in the database thusly:

[caption id="attachment_10824" align="alignnone" width="336" caption=""]<a href="http://youthtoday.vagrant.dev/files/2015/11/unnamed.png"><img src="http://youthtoday.vagrant.dev/files/2015/11/unnamed-336x67.png" alt="unnamed" width="336" height="67" class="size-medium wp-image-10824" /></a>[/caption]

So the missing is occurring after the database, but before caption_shortcode(). What else is on the img_caption_shortcode filter?

benlk commented 9 years ago

Just caption_shortcode:

var_log($wp_filter['img_caption_shortcode']);

array (
  10 =>· 
  array (
    '000000002b9bb4ba00000000cd3ae0f0caption_shortcode' =>· 
    array (
      'function' =>· 
      array (
        0 =>· 
        Navis_Media_Credit::__set_state(array(
        )), 
        1 => 'caption_shortcode',
      ),  
      'accepted_args' => 3,
    ),  
  ),  
), referer: http://youthtoday.vagrant.dev/wp-admin/post.php?post=10820&action=edit
benlk commented 9 years ago

Oh dear. It's actually nothing to do with Navis.

It's fixed if you comment out the regular expression so that largo_attachment_image_link_remove_filter( $post_content ) returns the original content in https://github.com/INN/Largo/blob/master/inc/images.php#L13

What is the purpose of removing links to attachments here?

benlk commented 9 years ago

@aschweigert Do you remember why we added this filter? https://github.com/INN/Largo/commit/bf169da17455667c96324e79b716931552c60882

benlk commented 8 years ago

The reason for losing everything on toggling between the Text editor and the HTML editor is that TinyMCE isn't looking for the credit or organizatino in the database, it's looking for them in the caption shortcode.

https://github.com/INN/Largo/blob/master/lib/navis-media-credit/js/media_credit_editor_plugin.js#L40-L60

When TinyMCE converts the HTML back to text, it doesn't have the organization or the credit, because it didn't find those in the generated shortcode that was inserted in the editor in the text editor.

Where does the shortcode come from?

benlk commented 8 years ago

Part of a possible solution: https://wordpress.stackexchange.com/questions/36568/solution-to-render-shortcodes-in-admin-editor

rclations commented 8 years ago

It looks like the navis media credit plugin is disabled for TinyMCE 4+, which means this issue likely appeared in WP 3.9 (April 2014) and later

rclations commented 8 years ago

Okay, so I've done a bunch of digging into WP Core files to understand what's happening here.

tl/dr;

Not sure how to make this work. WordPress filter order means that data is sent to TinyMCE before the credit field is saved.

Expected Behavior

The way this should work, is that inserting an image that contains a credit but no caption inserts the caption shortcode with an empty caption (allowing the shortcode to run, displaying the credit, but no caption).

Unexpected Result

When a new image is uploaded, and a credit is added (but no caption), the image is added to the wysiwyg editor without any caption, resulting in the credit not being shown on the front end.

Note: This only occurs under these exact conditions. Once an image has a caption already saved, it will display properly.

Why

The add_caption_shortcode function, called using the image_send_to_editor hook, has code that should be performing the necessary checks to add the [caption] shortcodes.

The issue is that WordPress core applies our filter and sends the code to the editor, and then, 9 lines later executes the function that contains our filter to save the credit field.

This means that there is no way to check if there is a credit saved in the database, which we need in order to determine whether to wrap the image in a caption shortcode (no need if there wasn't a credit added).

Ideas for next steps

We should definitely talk through this one, but my initial ideas for how we could make this work are

  1. Some javascript that detects and autosaves from the media modal window when things are entered - but even then, is that enough time to save to the db before we try to retrieve the data? Maybe we could save this to the browser itself?

Note: I tried the technique described here, but no luck.

  1. WordPress does have a credit field stored in the database in wp_postmeta under _wp_attachment_metadata. You can also see the field referenced in the codex. This appears to be for EXIF data from the image itself - but maybe we could build or utilize a plugin that allows editing of this exif data? That way we'd be using an existing WP Core structure. I'm not sure how this would play into our existing problem with the editor, but if we're using existing core structures, maybe it'll open up some new options.
benlk commented 5 years ago

classic editor: inserting an image with a credit but no caption results in no [caption] shortcode