Generate an error taxonomy for AMP error types
Closed this issue · 12 comments
As a user/developer, I should have a list of all the possible errors the AMP for WordPress plugin may generate and I should have them organized in a logical taxonomy (by CSS, by HTML attribute, by HTML element, or by Javascript). As a developer using the plugin, I should be able to use these taxonomies to sort and filter in various given views.
- AC1: Determine all current errors the sanitizer outputs.
- AC2: Organize this based on the logical taxonomy (by CSS, by HTML attribute, by HTML element, or by Javascript) discussed.
- AC3: Share this with the documentation for the new product site.
- AC4: Prepare this for the other views (index by error and single) where this will be used.
Question About Taxonomy
Hi @postphotos,
Thanks for opening this, and the clear ACs.
I should be able to use these taxonomies to sort and filter in various given views.
It sounds like this means a taxonomy in the WordPress sense.
Is there flexibility in implementing this? The amp_validation_error is a taxonomy itself.
Update: it looks like the answer to this question is "yes," based on this design.
Hi @postphotos,
Building on the question above, would there be a way to somehow filter by error type on the "AMP Validation Errors" page?
Hi @kienstra, that's the idea here. This ticket explores and implements the data model to allow the other views to have the features we think they need to help a user work with errors.
As a user, I should be able to get to just the errors of a given type (e.g. CSS or Javascript) to be able to see or action on just that subset of errors on both the URL and Error type screens.
On the single page, the new view also groups these errors by this typology, also linking to given documentation about how to best resolve it. For example, some markup attributes and CSS properties are just fine to be omitted in AMP, but I'd be wary of blocking some Javascript tags. In short, we want to use this data elsewhere in the tool and first need to make sure we can parse through (or collect it) as the sanitizer and plugin works.
@kienstra The current default post faceting (All | New | Rejected | Accepted) would be replaced with the new interface's dropdown filters.
Notes about the current version of the sketch:
Non-CSS Error Codes (that I could find so far)
'invalid_element'
(This code also looks to apply when enqueuing scripts or invalid stylesheets. In those cases, it would have a node_name of script or link, respectively.)
'invalid_attribute'
CSS Parser Error Codes
'css_parse_error'
'excessive_css'
'illegal_css_at_rule'
'illegal_css_important'
'illegal_css_property'
'removed_unused_css_rules'
'unrecognized_css'
'disallowed_file_extension'
'file_path_not_found'
The error codes, like invalid_attribute, are stored in the term description. That description is a JSON blob, like:
On the 'AMP Validation Errors' page, invalid_attribute and invalid_element codes also display the node_name after:
That node_name also looks to be in the JSON blob in the term description.
In regards to data model for a taxonomy of taxonomies, see #1361 (comment) for a discussion of a possible solution.
Nevertheless, it seems that doing a full-text search of the term description field (a JSON blob) is going to be the most practical solution here. This would mean that the error type (category) should be stored inside the JSON blob itself with the error code, and that to filter by this field it would require doing a query like the following, assuming there is a "css_error" type that is common category for all CSS-related validation errors:
WHERE description LIKE '%"type":"css_error"%';This would be in addition to the term status condition achieved via term_group.
This isn't ideal by any means, but I think it will be OK given that the type queries would only be done in the admin for these screens.
An important consequence of this implementation is this: we would not be able to readily limit the dropdown of type options to just those which actually exist in the database. Nor would we readily be able to provide a count of the given error types that exist with each option.
@westonruter, thanks a lot for your detailed suggestions for the data model.
Updated title to more clearly explain the intent of the ticket, props @amedina 👍
Change 'Trash' Text
On the 'Errors by URL' page, please change the 'Trash' text:
Request To Change Text
Hi @johnwatkins0,
Could you please change the text here on the 'Invalid AMP Pages (URLs)' page? The screenshot below has the same changes as the one above. But it's taken from my local environment, using the develop branch:
