Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide Block Suppression capability on AMP pages #5167

Closed
amedina opened this issue Aug 4, 2020 · 3 comments
Closed

Provide Block Suppression capability on AMP pages #5167

amedina opened this issue Aug 4, 2020 · 3 comments
Labels
Validation WS:Core Work stream for Plugin core

Comments

@amedina
Copy link
Member

amedina commented Aug 4, 2020

In 2.0 the Plugin Suppression capability allows users to suppress invalid markup from being injected to AMP pages, while keeping the plugin active for non-AMP pages. In some cases, when a feature of the offending plugin is used and then the corresponding plugin is suppressed, there may be content associated with the feature but it may not be what the user intended.

The plugin should offer the capability of removing certain blocks from an active plugin from being rendered on AMP pages.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

@amedina amedina added Validation WS:Core Work stream for Plugin core labels Aug 4, 2020
@amedina amedina added this to the v2.1 milestone Aug 4, 2020
@amedina amedina changed the title Provide capability of preventing all output from a given plugin on AMP pages Provide capability of suppressing specific blocks on AMP pages Aug 5, 2020
@amedina amedina changed the title Provide capability of suppressing specific blocks on AMP pages Provide Block Suppression capability on AMP pages Aug 5, 2020
@johnwatkins0
Copy link
Contributor

johnwatkins0 commented Aug 18, 2020

From what I understand after discussing this earlier, the acceptance criterion for this would be, "On AMP pages, all block markup associated with suppressed plugins is filtered out of the post content."

The challenges are:

  • Statically rendered Gutenberg blocks are saved into the database in the post content. So even where their plugin is suppressed their markup still renders.
  • If these blocks are registered entirely in JS (via registerBlockType) then there is no reliable way to find out which plugin they derive from while validating a URL or rendering post content. Blocks can be registered with any arbitrary name. This can make it difficult to detect their plugin sources.

So I think static block suppression could involve two levels:

  1. Static blocks whose invalidity derives from invalid markup can be captured during URL validation with the parse_blocks function. We can iterate through the blocks in the post's content and validate them individually. In this case, it might be best to not even try to associate these blocks with plugins. Instead, we can save the block name (parsed from the HTML comment) as the source of the error and make it suppressible by block name.
  2. When a statically rendered block has no invalid markup but depends on suppressed CSS/JS to function, then there may be nothing we can do. An exception is where blocks have a block.json file that can be detected and parsed, but this is probably getting into a very small number of cases, at least for now.

Number 1 is going to cover many cases because the best practice for registering custom blocks that depend on CSS/JS is to use register_block_type with the script and style args, but there plugins can obviously enqueue the scripts/styles for a block outside register_block_type, so we can't count on Number 1 100%.

@westonruter
Copy link
Member

  • Blocks can be registered with any arbitrary name. This can make it difficult to detect their plugin sources.

This is a current challenge for AMP validation as well. We can only report the block that an error is coming from in such cases, not the plugin that added the static block in the first place. So I believe this addresses the first level you called out.

For plugin suppression, there's also an open todo regarding static blocks:

/**
* Suppress plugin blocks.
*
* @todo What about static blocks added?
*
* @param string[] $suppressed_plugins Suppressed plugins.
*/
private function suppress_blocks( $suppressed_plugins ) {

For static blocks that have corresponding register_block_type() calls, then that could indeed be used as a way to identify which plugin is responsible for a given block. Bit that's more for Plugin Suppression than Block Suppression. If we wanted to suppress a block as a whole, then all we need is the block name regardless of the plugin. Nevertheless, at least in the case of dynamic blocks, I think most of the time it will be preferable for the static content contained inside the plugin's block to pass through as fallback content. Otherwise merely suppressing a block won't be enough: we should actually provide block substitution functionality, so that certain AMP-incompatible blocks are swapped with AMP-compatible ones on AMP pages. But that gets very complicated and I am uncertain if this makes sense to be part of the AMP plugin itself (for the 80% use case), or if it should be handled on a case-by-case basis by users who need it.

For example, I made a gist plugin which allows you to hide/show certain blocks based on whether the additional class names have amphtml or noamphtml provided. So with that you could have two blocks in the content, an AMP-compatible one with amphtml class name and an AMP-incompatible block with noamphtml provided. The user would see the two versions in the editor, but on the frontend they'd only see one displayed at a time based on whether they are looking at an AMP page.

@westonruter westonruter removed this from the v2.1 milestone Oct 8, 2020
@milindmore22
Copy link
Collaborator

A Support topic, requesting to add functionality to hide/suppress part of the content on AMP endpoint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Validation WS:Core Work stream for Plugin core
Projects
None yet
Development

No branches or pull requests

4 participants