Dominant color upload process ignores potential third-party editors

[felixarntz]

Bug Description

Follow-up to #19 / #282: The current Dominant Color module enforces usage of its extended WP_Image_Editor classes. While this works with WordPress core alone, it is problematic in environments where e.g. a plugin has already added its own extended WP_Image_Editor classes.

We need to make sure that the Dominant Color implementation doesn't conflict with such plugins. The dominant_color_set_image_editors() filter callback function should be fixed to not override the entire array of editor class names, but rather only replace the specific WordPress core editors WP_Image_Editor_GD and WP_Image_Editor_Imagick with their customized variants from the module.

cc @pbearne @spacedmonkey

[spacedmonkey]

Before calling the methods, we do check to see if they exist. See.

performance/modules/images/dominant-color/load.php

Lines 306 to 308 in 11bf31c

	if ( ! method_exists( $editor, 'has_transparency' ) ) {
	return new WP_Error( 'unable_to_find_method', __( 'Unable to find has_transparency method', 'performance-lab' ) );
	}

performance/modules/images/dominant-color/load.php

Lines 274 to 276 in 11bf31c

	if ( ! method_exists( $editor, 'get_dominant_color' ) ) {
	return new WP_Error( 'unable_to_find_method', __( 'Unable to find get_dominant_color method', 'performance-lab' ) );
	}

If you have filter the editor class to a custom one, these methods will not exist and nothing will happen.

[felixarntz]

@spacedmonkey That's not the point I'm making here. The current filter overrides all registered WP_Image_Editor classes - which is a problem if other plugins have added their own ones.

The dominant_color_set_image_editors() function needs to be adjusted to not "recklessly" overwrite all classes. It should only replace the specific WP core image editor classes WP_Image_Editor_GD and WP_Image_Editor_Imagick, but avoid the risk of unsetting/modifying image editor classes potentially added by plugins.

[spacedmonkey]

This is only really an issue in this plugin. Once in core, it will not be a problem. But I agree, we want to not break anything for anyone doing something really custom.

[felixarntz]

@spacedmonkey

This is only really an issue in this plugin. Once in core, it will not be a problem.

Correct, but I think that should be enough of a reason to fix this. We shouldn't have known bugs in this plugin without addressing them, since this plugin is also used in the wild. So we should still do our best to have it working as reliably as possible.

But I agree, we want to not break anything for anyone doing something really custom.

Exactly :)

[spacedmonkey]

PR ready to review.