Display error when a plugin is activated which tries to do something incompatible with AMP

[MackenzieHartung]

Acceptance Criteria

AC1: On activating a plugin on wp-admin/plugins.php, if the plugin does something that’s not compatible with AMP, display an error at the top of the page
AC2: The error message will display the following text:. “ is not compatible with AMP, and could result in content displaying improperly on your site.”

Tasks

• Add a "reporting" flag whenever something is removed by the output buffer sanitizer and use that to build the "Bad Plugin" error.
• Report other things than just node removal, including enqueued scripts and styles.
• Report errors related to unknown sources.
• Add method to disable AMP sanitization to be able to debug what is going on?
• Improve initialization process for how validation is requested.

[kienstra]

Question About Approach

Hi @westonruter,
Using the sanitization reporting PR, what do you think about this approach:

1. Before activating a plugin on /wp-admin/plugins.php,
2. Make a request for the header of a single post page, like https://local.envi/?p=1633
3. Store the AMP-Validation-Error from the header. Example:

{
"has_error": true,
"removed_nodes": {"script": 1},
"removed_attributes": null
}

4. After the plugin activation, make another request like in step 2, and compare the AMP-Validation-Errorvalues.
5. If there are 'removed_nodes' in step 4 that weren't present in step 2, display an error message.
6. Possibly display in the notice the node(s) that the plugin added that the sanitizer removed.

Thanks, Weston 😄

[westonruter]

@kienstra I think that this will not give us complete results. A plugin may not do anything upon activation. It may require a user to actually do something, like add a widget or shortcode.

I think the best we can probably do here is:

1. Watch for a plugin enqueueing a script or stylesheet. If a script or stylesheet gets removed by sanitizer, look for the wp-content/(themes|plugins)/(.+) directory the file is located in. And this would indicate which plugin or theme is doing something bad.
2. And this is going to be more experimental, but we could maybe iterate over all of the $wp_filter->callbacks to find all added callbacks that are sourced from a theme or plugin. For example, an instance of a class could have its location looked up via ReflectionClass::getFilename(). Then for each one, we could wrap the callback with our own callback which then does output buffering before and after, and then we could look at the output to see if it contains anything invalid. It may be best to output some kind of HTML comment, like before and after (<!--before:jetpack--> ... <!--after:jetpack-->) and then later use the sanitizer to keep track of which plugin is was running at the time a node was removed.

[westonruter]

Both of these checks would only be done when explicitly requested, especially the latter one because I imagine it would be expensive.

[westonruter]

It should also be possible to use the similar approach for shortcodes. We could wrap each registered shortcode handler with a function that puts those milestone comments before and after. The sanitizer would then be able to know which shortcode was responsible for the illegal output. The specific theme or plugin could be identified via the Reflection API to fond the filename that contains the function or class.

[kienstra]

Discussion Is On PR

Most of the discussion and solution design for this issue is now taking place on this pull request.

[kienstra]

Steps To Test

Hi @csossi,
Could you please test this?

plugins.php

1. Go to the dev site /wp-admin/plugins.php page.
2. Deactivate and activate the (test) plugin 'AMP Invalid.'
3. Expected: the warning includes the name amp-invalid

4. Expected: clicking 'More details' drives to the 'Validation Status' page

Validation Status Page

1. On the dev site /wp-admin section, go to 'AMP' > 'Validation Status.'
2. Expected: each validation error has value(s) for 'Title,' 'Count,' 'Removed Elements,' 'Removed Attributes,' 'Incompatible Sources,' and 'Date.' The value '--' is acceptable. And the 'badge' has the number of error posts:

3. Observe a plugin that's in the 'Incompatible Sources' column (shown below)
4. Deactivate that plugin
5. Click 'Recheck'

6. Expected: that plugin no longer appears in the 'Incompatible Sources' column for that error. It may appear in other error posts.
7. Reactivate the plugin
8. Repeat Steps 1-7 above. But instead of clicking 'Recheck' inline,' select a few boxes and click 'Recheck' from 'Bulk Actions'

9. Expected: same as 6. above, but the deactivated plugin shouldn't appear in any of the rechecked errors.
10. Go to the dev site dashboard
11. Expected: the 'At a Glance' widget shows a message with the amount of error posts:

Single Error post.php Page

1. From the Validation Status page above, click 'Details' after hovering over one of the post titles.
2. Find a plugin that's listed in the 'Sources' section, like 'amp-invalid':

3. Deactivate that plugin.
4. Click 'Recheck on this page (shown above)
5. Expected: that plugin is no longer listed in that post. It's possible that this redirects to the 'Validation Status' page if the page no longer has any error.

Troubleshooting
If there aren't any errors on the Validation Status page, deactivate and activate the 'AMP Invalid' plugin.

[csossi]

verified in QA