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

Remove validation handling options with the checkbox to disable auto-accepting sanitization #3350

Closed
schlessera opened this issue Sep 26, 2019 · 1 comment · Fixed by #3545
Assignees
Labels
Enhancement New feature or improvement of an existing one Sanitizers
Milestone

Comments

@schlessera
Copy link
Collaborator

schlessera commented Sep 26, 2019

For the vast majority of users the option to auto-accept validation errors should be enabled:

image

This option is in fact enabled by default. Based on usage over the past year it seems this option is rarely unchecked, and in fact it is not even able to be unchecked when in Standard mode:

image

Therefore the checkbox option should be removed entirely.

As part of this, the notices that appear in the validated URLs screen should be removed:

image

image

Note that in #3346 (fixing #2326) the excessive_css validation error actually defaults to auto-rejected when the site is in Standard mode. To extend this now to give control over a site's developers to have control over the default behavior, the AMP_Validation_Manager::is_sanitization_auto_accepted() method should be made filterable. This will then allow for the removal of the checkbox and removal of the 'auto_accept_sanitization' option.

Context: For almost all users, accepting sanitization by default just makes sense, so going by the "decisions, not options" mantra of WordPress and removing the option seems sensible. See #3346 (comment)

This is related to #2346 which removed the checkbox for showing the admin bar in AMP pages. It reverts the admin notices added in #1376.


Whether a new validation error is sanitized by default is filterable via amp_validation_error_default_sanitized. For example: https://gist.github.com/westonruter/c1496d668b2a73a44aa423e6547a59b7.

@schlessera schlessera added Enhancement New feature or improvement of an existing one Sanitizers labels Sep 26, 2019
@westonruter westonruter added this to the v1.3.1 milestone Oct 8, 2019
@westonruter westonruter changed the title Make AMP_Validation_Manager::is_sanitization_auto_accepted() filterable Remove validation handling options with the checkbox to disable auto-accepting sanitization Oct 8, 2019
@westonruter
Copy link
Member

I've updated the title and description to combine the removal of the auto-accept checkbox with the addition of the filter for whether a validation error's sanitization is auto-accepted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or improvement of an existing one Sanitizers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants