Skip to content

Suppress plugins early when making a Reader theme request#5009

Merged
westonruter merged 3 commits intodevelopfrom
add/reader-theme-early-plugin-suppression
Jul 9, 2020
Merged

Suppress plugins early when making a Reader theme request#5009
westonruter merged 3 commits intodevelopfrom
add/reader-theme-early-plugin-suppression

Conversation

@westonruter
Copy link
Copy Markdown
Member

@westonruter westonruter commented Jul 9, 2020

Summary

Fixes #4477.
Amends PR #4657

I realized that when a Reader theme is selected, accessing the AMP Customizer currently results in Customizer controls et al being registered even for plugins that are suppressed. The reason for this is that suppression was happening at the wp action which is too late (and never even fires) when accessing customize.php. Therefore, this PR fixes that problem by checking if the current request is for an AMP Reader theme (whether on the frontend or the Customizer) and, if so, suppresses plugins immediately at plugins_loaded as early as possible.

This early-suppression could also be done in legacy Reader mode or Transitional mode since there is a query var which indicates it is an AMP request. Nevertheless, this is unnecessary to do because neither Transitional mode nor legacy Reader mode have separate Customizers. Therefore, only “Reader theme” mode needs this early suppression.

To test this:

  1. Activate Reader mode and select a Reader theme.
  2. Install the latest version of my my Bad Block plugin: https://gist.github.com/westonruter/e4331b3f6d03aac075b86b226acd3ba5
  3. Add the Bad block to a post.
  4. View the AMP page.
  5. Click on Customize to open that AMP URL in the AMP Customizer.
  6. Modify the "Bad Block Background Color" in the Colors section.
  7. Validate the AMP URL (again).
  8. Navigate to AMP settings and suppress Bad Block from running.
  9. Navigate back to the AMP page and verify the block is not outputting anything.
  10. Open the AMP Customizer for that post and verify the Customizer control for Bad Block Background Color is absent.
Before After
before after

(The screenshot isn't correct as the Before shouldn't be showing the bad block!)

Checklist

  • My pull request is addressing an open issue (please create one otherwise).
  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@westonruter westonruter added this to the v1.6 milestone Jul 9, 2020
@westonruter westonruter requested a review from pierlon July 9, 2020 03:38
@googlebot googlebot added the cla: yes Signed the Google CLA label Jul 9, 2020
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jul 9, 2020

Plugin builds for 6898ab1 are ready 🛎️!

@westonruter westonruter added the Bug Something isn't working label Jul 9, 2020
Co-authored-by: Pierre Gordon <16200219+pierlon@users.noreply.github.com>
@westonruter westonruter merged commit 1adf204 into develop Jul 9, 2020
@westonruter westonruter deleted the add/reader-theme-early-plugin-suppression branch July 9, 2020 07:06
@schlessera
Copy link
Copy Markdown
Collaborator

QA passed

The orange background is gone on the AMP page:

Image 2020-07-16 at 6 41 37 PM

The orange background is also gone on the Customizer screen:

Image 2020-07-16 at 6 41 30 PM

@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Jul 18, 2020
@westonruter westonruter added the WS:Core Work stream for Plugin core label Aug 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something isn't working Changelogged Whether the issue/PR has been added to release notes. cla: yes Signed the Google CLA WS:Core Work stream for Plugin core

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gracefully handle plugins that cause validation errors

4 participants