Embedly, oEmbed, s2Clean, and WPKBA

[jaswrks]

@raamdev writes...

I'm testing oEmbeds in WP KB Articles on ZenCache.com and I discovered that placing, for example, a YouTube video link on a line by itself does not get picked up by WordPress and turned into the embedded video. However, if I use [embed]<link>[/embed] it works just fine.

I tested this on ZenCache.com using both a regular Post and a KB Article post. Here's the test post on ZenCache.com. If you can confirm this sounds like an s2Clean bug, I'll open a GitHub issue.

@jaswsinc writes...

---- I don't believe it's a bug. It's a feature that I need to document better. Give me just a second and I'll try to get my thoughts together and review the code again.

---

@jaswsinc writes...

I'm taking a closer look at this. The s2Clean theme comes with Embedly. This is not necessarily a bug, because I remember noting it here in the source code, that [embed] should be used to call upon the WordPress embed system.

However, I do think there is room for improvement, on both the s2Clean and WPKBA side. Using Embedly for things like YouTube videos in KB articles is probably not ideal for us, and there should be more flexibility with this. In the case of KB articles written in Markdown, I think the WPKBA plugin should also come with an option to automatically convert those into [embed] for improved compatibility with themes/plugins that might alter content in ways that require the more reliable [embed] shortcode syntax.

Ultimately, in KB articles I'd like to avoid the need of typing [embed] each time. Just to keep it simple. That we can simply place the URL on a line of its own.

That should already work with s2Clean and Embedly. I think the only reason it's not is that we have Embedly enabled at ZenCache.com with the autoload option. If you stick the word embedly into the article it should start working. Or, if we configure s2Clean to load embedly always, that would resolve it in the short term also. Another solution (which Raam already noted) is to use the shortcode syntax.

[raamdev]

What's the reason s2Clean can't just use the oEmbed features provided by WordPress itself? Does this have something to do with those features conflicting with features of s2Clean?

I've personally never heard of Embedly; why would I want to use that over the regular oEmbed features of WordPress?

If there was a way to disable Embedly functionality (assuming that's what's introducing the s2Clean conflict with WordPress oEmbed functionality) so that I could use what's provided by WordPress (i.e., paste the URL on a line by itself), that would be preferable in my mind.

[jaswrks]

I agree. I'm not sure why I disabled it now. I remember there was a good reason for it, but I don't remember now what it was exactly. It's something that I'd like to look at again.

Ideally, we could overcome whatever the issue was previously and get that working. I think it probably had something to do with hook/filter priorities and the loading order of the WP oEmbed causing conflicts with the large mixture of syntaxes that we have when supporting shortcodes, raw HTML, Markdown, together with oEmbed; all at the same time.

[jaswrks]

I've personally never heard of Embedly; why would I want to use that over the regular oEmbed features of WordPress?

Embedly supports a much larger range of services with a lot more features. It's used inside Slack, for instance. It's not something that you hear about often, but it powers most of the oEmbed that we see across many services. http://embed.ly/

It's not ideal in all cases though, because it has limits. For KB article videos it seems like the wrong choice in my view. I'd rather use the default WP oEmbed system too.

[jaswrks]

Just wanted to clarify that s2Clean only alters content filters in WordPress whenever Markdown processing is enabled. So reproducing this issue requires that Markdown be enabled, which triggers a bunch of changes to the WP content filters when it's on.

[raamdev]

s2Clean only alters content filters in WordPress whenever Markdown processing is enabled

Ah, that's clarifies a few things. :)

Now that the Jetpack plugin comes with a Markdown processor, is there any reason to have a custom Markdown processor?

[jaswrks]

Now that the Jetpack plugin comes with a Markdown processor, is there any reason to have a custom Markdown processor?

I think it is a nice feature that we can offer with s2Clean. In my case, I am using s2Clean's Markdown functionality on a variety of sites. I like the flexibility of being able to tweak it and tune it easily as part of our product. It supports Markdown Extra also, which is something that I'm using at jaswsinc.com.

I haven't played with the MD functionality in JetPack though. Does it support Markdown Extra? Or does it provide options for what sort of Markdown flavor/parser is used? Does it support raw HTML and shortcodes as part of the Markdown syntax?

Just asking in case you have experimented with it. I haven't yet.

[raamdev]

Just asking in case you have experimented with it. I haven't yet.

I haven't explored its edges at all, no. But then I have very simple uses. I like being able to write bold, italics, headers, lists, and links in Markdown and I'm already so used to it that it's nice to know I can copy/paste that into WordPress and have it just work. However, I haven't done much with mixing in shortcodes, etc. There are no options, as far as I can tell, regarding the type of Markdown it supports.

In any case, this is probably a discussion for another GitHub issue. :)

Regarding s2Clean and its built-in Markdown support: I think it's great that it has a powerful Markdown processor and I feel like that definitely adds to the feeling of it as a Pro product. However, what would counter that and detract from those features making it feel like a Pro product would be if they conflicted with some of the most common features used by WordPress users (the simple oEmbed via pasting a link is one example). If the s2Clean Markdown features caused all kinds of issues (or maybe even broke) when Jetpack's Markdown feature was enabled, that would make it feel less like a polished Pro product.

At the bare minimum, I would expect s2Clean (as a Pro product) to work flawlessly with all of the Jetpack features (because Jetpack is very popular), and with any features that WordPress natively supports. If that means dynamically disabling some s2Clean features when it detects Jetpack is active for example, that's fine (that's how I handle potential conflicts in the Independent Publisher theme).

[jaswrks]

Referencing hackety code from the WP core that is indirectly related to this conflict. This, combined with clickable() that is supported by s2Clean's Markdown implementation is the reason for the current limitation whenever Markdown is enabled together with Embedly.

class WP_Embed {
    public $handlers = array();
    public $post_ID;
    public $usecache = true;
    public $linkifunknown = true;

    /**
     * When an URL cannot be embedded, return false instead of returning a link
     * or the URL. Bypasses the 'embed_maybe_make_link' filter.
     */
    public $return_false_on_fail = false;

    /**
     * Constructor
     */
    public function __construct() {
        // Hack to get the [embed] shortcode to run before wpautop()
        add_filter( 'the_content', array( $this, 'run_shortcode' ), 8 );

        // Shortcode placeholder for strip_shortcodes()
        add_shortcode( 'embed', '__return_false' );

        // Attempts to embed all URLs in a post
        add_filter( 'the_content', array( $this, 'autoembed' ), 8 );

        // After a post is saved, cache oEmbed items via AJAX
        add_action( 'edit_form_advanced', array( $this, 'maybe_run_ajax_cache' ) );
    }

    /**
     * Process the [embed] shortcode.
     *
     * Since the [embed] shortcode needs to be run earlier than other shortcodes,
     * this function removes all existing shortcodes, registers the [embed] shortcode,
     * calls {@link do_shortcode()}, and then re-registers the old shortcodes.
     *
     * @uses $shortcode_tags
     * @uses remove_all_shortcodes()
     * @uses add_shortcode()
     * @uses do_shortcode()
     *
     * @param string $content Content to parse
     * @return string Content with shortcode parsed
     */
    public function run_shortcode( $content ) {
        global $shortcode_tags;

        // Back up current registered shortcodes and clear them all out
        $orig_shortcode_tags = $shortcode_tags;
        remove_all_shortcodes();

        add_shortcode( 'embed', array( $this, 'shortcode' ) );

        // Do the shortcode (only the [embed] one is registered)
        $content = do_shortcode( $content );

        // Put the original shortcodes back
        $shortcode_tags = $orig_shortcode_tags;

        return $content;
    }

[jaswrks]

Referencing this method in the s2Clean codebase which is currently unused. It appears that I considered this before and created a method to deal with it, but never followed through on actually implementing it.

https://github.com/websharks/s2clean-pro/blob/000000-dev/s2clean/includes/functions.php#L2854

[jaswrks]

Embedly (if enabled) is now secondary to WordPress oEmbed syntax when Markdown processing is enabled with s2Clean. Without MD processing enabled, there are no known conflicts since the filter adjustments are not applied in that scenario.

@raamdev I will let you close this out once you can confirm that the issue has been resolved at ZenCache.com. I have updated that site to the latest jaswsinc branch of the s2Clean theme which contains the fix.

[raamdev]

I will let you close this out once you can confirm that the issue has been resolved at ZenCache.com

Confirmed fixed. Thank you!

[jaswrks]

@raamdev: I remember seeing a comment from you the other day about a YT video not working as expected, and then suddenly it appeared. It looks like the video is being parsed by Embedly instead of giving precedence to the oEmbed parser in WordPress.

I see this is the reason the issue has come back again...

[jaswrks]

A possible solution would be for WPKBA to run the article content through WP oEmbed filters prior to parsing the Markdown. That seems like it would be the best way to handle this

I need to build a chart that shows how all of this works in the various scenarios that can play out.

[jaswrks]

[raamdev]

It looks like the video is being parsed by Embedly instead of giving precedence to the oEmbed parser in WordPress.

Ah, that would explain it! Yeah, I've been seeing that slight delay in loading on all the ZenCache KB Articles I recently published. For some articles (the ones with plenty of other non-video content), the slight delay isn't that bad. But for others, where the only content in the KB Article is a video link, the article temporarily looks empty before the embed gets loaded.

[jaswrks]

I forgot to note something important in this diagram. This is what makes the s2Clean Markdown parser so awesome; is that it can handle mixed modes without conflict.

[jaswrks]

Shows which default content filters are removed from the WP core when Markdown parsing is enabled.

[jaswrks]

[raamdev]

@jaswsinc Awesome. Thanks so much for those diagrams! Really helpful.

We should move those (or some of them; whatever you think is appropriate) to a WP KB Articles KB Article.

I would go ahead and do this now, but I'm not sure if any of that stuff is in flux right now with you working on it. :-)

[jaswrks]

I would go ahead and do this now, but I'm not sure if any of that stuff is in flux right now with you working on it. :-)

I agree, that would be awesome for us to do that. Let me run those through one more round of updates and see if I can enhance them a bit further. I'll post an update here when they are complete. There are still some details missing that I'd like to include.

[jaswrks]

WPKBA filters explained here: https://github.com/websharks/wp-kb-articles/issues/67 Diagram (final draft).

[jaswrks]

s2Clean filters explained (final draft).