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

Add support for also_requires_attr validator constraint #4447

Closed
westonruter opened this issue Mar 26, 2020 · 0 comments
Closed

Add support for also_requires_attr validator constraint #4447

westonruter opened this issue Mar 26, 2020 · 0 comments
Labels
P2 Low priority Validation WS:Core Work stream for Plugin core

Comments

@westonruter
Copy link
Member

Feature description

AMP requires that any element with an on event handler attribute also have a tabindex and a role=button. Currently, however, you can create AMP content in WordPress and validation errors will leak.

Consider a Custom HTML block containing:

<span on="tap:AMP.print">Print!</span>

Two validation errors leak to the frontend:

image

The reason for this the AMP plugin does not yet support the trigger like the on attribute spec has:

  attrs: {
    name: "on"
    trigger: {
      if_value_regex: "tap:.*"
      also_requires_attr: "role"
      also_requires_attr: "tabindex"
    }
  }

There are currently 22 instances of trigger in the protoascii, and each one is for also_requires_attr. This is currently called out as a todo:

* @todo Need to check the following items that are not yet checked by this sanitizer:
*
* - `also_requires_attr` - if one attribute is present, this requires another.
* - `ChildTagSpec` - Places restrictions on the number and type of child tags.
* - `if_value_regex` - if one attribute value matches, this places a restriction
* on another attribute/value.

The tag-and-attribute sanitizer needs to be updated to support this constraint. This will include updating the Python spec parser to extract these values. When a failure is encountered a new validation error code should be raised. (Currently it is UNKNOWN in the AMP validator but we can do something specific.)


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

@westonruter westonruter added Validation P1 Medium priority labels Mar 26, 2020
@westonruter westonruter added this to the v1.6 milestone Apr 4, 2020
@westonruter westonruter added P2 Low priority and removed P1 Medium priority labels Apr 12, 2020
@kmyram kmyram modified the milestones: v1.6, Sprint 28 Apr 14, 2020
@kmyram kmyram modified the milestones: Sprint 28, v1.6 May 27, 2020
@westonruter westonruter removed this from the v1.6 milestone Jun 15, 2020
@kmyram kmyram added the WS:Core Work stream for Plugin core label Aug 5, 2020
@westonruter westonruter closed this as not planned Won't fix, can't repro, duplicate, stale Jul 26, 2022
@github-project-automation github-project-automation bot moved this to Backlog in Ongoing Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 Low priority Validation WS:Core Work stream for Plugin core
Projects
Archived in project
Development

No branches or pull requests

2 participants