ResponsiveImagesCG / wp-tevko-responsive-images

Fully responsive image plugin for wordpress
449 stars 53 forks source link

Ensure compatibility with plugins that replace WP's default WYSIWYG editor #160

Closed tevko closed 9 years ago

tevko commented 9 years ago

In some cases, users can run into a bug when using a wysiwyg editor that is not the default tinyMCE editor provided by wordpress. This is most likely due to a javascript error, in that our plugin javascript isn't triggered by non-default wysiwyg editors. Here's an example of such - https://wordpress.org/support/topic/srcset-not-updating-when-image-is-replaced-in-black-studio-tinymce-widget-editor?replies=1

joemcgill commented 9 years ago

This issue will become moot if we switch to content filtering (re: #144).

joemcgill commented 9 years ago

Any external editor that applies filters attached to the_content filter hook should now automatically have srcset and sizes attributes added to images.

See #177

marcochiesi commented 9 years ago

Would you consider adding the filter also to the widget_text hook? It is used by the native WP text widget and also by other plugins like Black Studio TinyMCE Widget, which was mentioned at the beginning of this thread.

joemcgill commented 9 years ago

Hi @marcochiesi. Thanks for the suggestion, it's definitely worth considering. I'm curious why you don't use the filters associated with the_content rather than the widget_text filters in your plugin, since you're including a full TinyMCE editor and not just a textarea input like the default text widget?

marcochiesi commented 9 years ago

The the_content filter is used to filter the content of posts, but when you use a widget there's no post associated with it. Often plugins that hook to the_content assume that there's a current post and they use post-related stuff, which is not relevant in a widget context (i.e. global $post variable or other functions to accessa data of the current post). Moreover, the plugin is a replacement for the native WordPress text widget, which uses the widget_text filter, so every 3rd party script made for the native text widget will work out of the box.

joemcgill commented 9 years ago

Got it. It would make sense that some filters would assume a global $post object. To be honest, I'm not sure if we should add this filter to text_widget by default or not, since most text widgets don't include a UI for embedding images directly.

Additionally, if this functionality gets merged into WordPress—which is the current goal—it would be better for you to apply the filter in the context of the plugin, rather than having WordPress automatically run all text widgets through this particular filter, in my opinion.

tevko commented 9 years ago

@joemcgill as @marcochiesi mentioned, could cause some issues if a user is uploading images through a plugin like ACF?

joemcgill commented 9 years ago

I don't think it would cause problems, but there might definitely be edge cases where filtering all text_widget content would be undesirable.

marcochiesi commented 9 years ago

If the code will get merged into WordPress core, I would definitely add the filter on the Black Studio TinyMCE Widget side. I suppose that the function name will change once it gets merged... isn't it?

joemcgill commented 9 years ago

I suppose that the function name will change once it gets merged.

That's correct. I'll update you once we finalize the new filter name.

joemcgill commented 9 years ago

Leaving this open until we finalize the new filter name. Changing status from bug to question.

joemcgill commented 9 years ago

Hi @marcochiesi,

Looks like the core display filter is going to be named wp_make_content_images_responsive(). You can now test this on a development version of WordPress: https://github.com/wordpress/wordpress.