Deactivating plugin makes images expand to 100%

[joemcgill]

An important issue was raised in the WP.org support forums, where turning off the plugin results in markup where a srcset attribute is present, but not a sizes attribute because we are adding a data-sizes attribute instead and transforming those to sizes attributes on the front end.

This will no longer be an issue going forward once #177 lands, but we should provide documentation for folks who encounter this problem in the future.

[jaspermdegroot]

Might be good f we start working on a tool that removes the markup which was inserted by the plugin. If it's not for when we switch to content filtering, then it can be used for cleanup after deactivating the plugin.

I guess/hope a lot of the code from #148 can be used for this. Just have to make it remove srcset and data-sizes attributes instead of adding them,

[joemcgill]

A WP-CLI option would be great, but I wonder how many people will know how to make use of a CLI tool for this type of job? Another option would be to run the cleanup script ourselves through a plugin deactivation hook (https://codex.wordpress.org/Function_Reference/register_deactivation_hook). Either way, we'll need to carefully test any solution that edits people's content so we don't accidentally break things.

[jaspermdegroot]

I think this is our main priority for 3.2.
We could write code to remove srcset and data-sizes attributes from image elements in the content in the database, but another option is to provide clear instructions how to do it with a plugin like https://wordpress.org/plugins/better-search-replace/ or https://wordpress.org/plugins/search-regex/.
We could use the plugin deactivation hook to show a message, but I don't think we should automatically run something. People should backup first and you should only do it if you have used the plugin prior to version 2.5.

[joemcgill]

Thinking a little more about this, now that srcset is added in core by default, the possible damage here is quite small. In fact, this issue will only happen when people have specifically removed height and width attributes from the image markup in post content. Maybe in core we can check for any srcset on post content that uses w descriptors and generate the sizes attribute if it isn't already present rather than short circuiting both. The data-sizes attribute would still be present, but I would prefer that to accidentally breaking someone's content with a database rewrite.

Thoughts?

[jaspermdegroot]

Thinking a little more about this, now that srcset is added in core by default, the possible damage here is quite small.

I don't see how that changed the situation. If you used the plugin before version 2.5 you have a srcset and data-sizes attribute in your markup. Core doesn't do anything when there is already a srcset, so disabling the plugin still results in no sizes attribute and the damage is still the same.

Maybe in core we can check for any srcset on post content that uses w descriptors and generate the sizes attribute if it isn't already present rather than short circuiting both.

I don't think we should make changes in core to fix a problem caused by this plugin. Specially not if it affects performance. We already had a discussion about whether or not to check if sizes is present before adding it, while sizes without srcset is only incomplete markup. Based on that I don't think a change like this will land in core since a srcset with w descriptor without sizes is invalid markup.

The data-sizes attribute would still be present, but I would prefer that to accidentally breaking someone's content with a database rewrite.

I am also hesitant when it comes to making the plugin doing a database rewrite. That's why I was thinking that it might be better to use another plugin that is around for a while already and has positive reviews instead of writing our own code for this.

My opinion is that it's a plugin's responsibility to leave things in the same state as it was before using it if someone disables the plugin. If it was just a harmless data-sizes attribute that we leave behind it would probably be acceptable, but we leave people with invalid markup that break things. Even if we make a change now in core that solves the problem, that code might change in the future and reintroduce the issue. So I think it's better if we handle this ourselves without depending on core.

[joemcgill]

I don't see how that changed the situation.

My late night brain was broken. Not sure what I was thinking here 😊.

this issue will only happen when people have specifically removed height and width attributes from the image markup in post content

That part is correct, and something that hadn't fully sunk in until I reread this ticket. Originally, I was thinking that everyone who turned off our plugin would be affected by this.

I don't think we should make changes in core to fix a problem caused by this plugin.

I think I agree, though I wonder if it would be beneficial for core to add sizes attributes when someone adds an inline srcset attribute using w descriptors for reasons beyond this plugin.

So I think it's better if we handle this ourselves...

Let's work on an approach that can be as automatic as possible but requires users to take intentional action (e.g., click a confirmation button) before running any scripts against their database.

[jaspermdegroot]

Let's work on an approach that can be as automatic as possible but requires users to take intentional action (e.g., click a confirmation button) before running any scripts against their database.

Sounds good.

Instead of telling people to use a search-replace plugin we could write the database rewrite script ourselves and ask the authors of those two plugins I linked to in my previous comment to review our code.

[jaspermdegroot]

Since we can only run one callback function upon plugin deactivation it's not possible to have users first make a backup and then run the code to remove data-sizes and srcset from images in the post content.
PR #271 is solution that offers a tool to remove the attributes while the plugin is still active and a warning message to point users to this tool. Both only if we found images with data-sizes and srcset attributes in post content.