Skip to content
This repository has been archived by the owner on Apr 2, 2020. It is now read-only.

Transform <img> to [img] on the fly with shortcode reversal #38

Open
danielbachhuber opened this issue Jun 3, 2015 · 7 comments
Open

Comments

@danielbachhuber
Copy link
Contributor

We should have some option for transparently converting <img> to [img] on the fly using shortcode reversal when the post is saved.

@vralle
Copy link

vralle commented Nov 20, 2015

My custom converter via DOMDocument: https://gist.github.com/vralle/1f9a79a42d0c35f3b00f
Code written for their own needs, but small changes would make it work for the plugin

@vralle
Copy link

vralle commented Nov 22, 2015

The link above has added new code.
Now we can check the src value. If the appropriate file is found in the media library, the converter returns ID attribute and size attribute instead of the src attribute

@goldenapples
Copy link
Contributor

@vralle - nice, thanks!

There are a few things in there that I don't think we can really depend on in many production environments... like being able to query for all attachments at once and loop through them. But something along those lines will be necessary in order to get beyond the naive regex that we're currently using to match image tags to the attachment ids.

I do also like the looping through the generated attachment sizes to try to match the filename of the img src to the filename of a specific crop size. I think we can probably do something like that in any migration/reversal script that lands in here.

Do you have time to try and put together a pull request that does this? The pattern we've been following with Shortcake is to include a "reversal" function (stubbed out here, which is hooked onto pre_kses and, taking the content string as input, returns the content with all tags that can be replaced with shortcodes replaced.

@vralle
Copy link

vralle commented Nov 23, 2015

Now the code is just a draft. It works, but it has a number of bottlenecks.

The main bottleneck - is a request for all attachments. There is an idea to create a request in a separate function and put the result into the cache by the Transients API. This has a significant impact on performance.

As I was not able to resolve the filter 'content_save_pre'. For unknown reasons, the first is used sanitize and does not depend on the priority specified for 'content_save_pre'

@vralle
Copy link

vralle commented Nov 27, 2015

use the Transients API

down

content_save_pre

now use 'edit_post_content'

Need to more testing

@vralle
Copy link

vralle commented Dec 2, 2015

  • get_attachment_id( $url ) tested and work fine
  • New Converter 'shortcode to html'.
  • Converters is compatible with WP Media editor (information transmitted through attachments classes)

@goldenapples
Copy link
Contributor

hmm. This is an interesting approach. So you have the post content converted into <img> tags when being edited, so that WordPress's native image controls can be used for editing, and then converted back into [img] shortcodes when being saved.

I don't think that's exactly what @danielbachhuber meant in the original issue. Since we're trying to build out an extensible UI for managing images, I think we'd rather use the Shortcake UI for managing images, and try to bring that into feature parity with the WordPress media UI, than just fall back to the WordPress UI. The issue here is that if we add new features to the [img] shortcode and its UI, say maybe adding a "credit" field with a filter on img_shortcode_ui_args, that UI won't be available in the WP editor... I think ultimately the goal should be to replace the core image

A few specific thoughts on your code:

  • The shortcode2html() function seems to needlessly duplicate the existing Img_Shortcode::callback() function. Is it necessary to have two separate functions which render the shortcode?
  • I'm still concerned about performance of the get_attachment_id() function. Its not rare at all in production for sites to have hundreds of thousands of images. That would (a) make for a massive entry into the wp_options table when setting the transient, and (b) mean potentially hundreds of thousands of databases queries on saving a post, as you loop through them all and get post meta for each. That could bring down a site very easily. I'm not sure of an easy way around that, but it looks like a problem to me.

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

No branches or pull requests

3 participants