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

Intent to Implement: CSS @media query validation #18246

Closed
Gregable opened this issue Sep 20, 2018 · 11 comments
Closed

Intent to Implement: CSS @media query validation #18246

Gregable opened this issue Sep 20, 2018 · 11 comments
Assignees
Labels
INTENT TO IMPLEMENT Proposes implementation of a significant new feature. https://bit.ly/amp-contribute-code P3: When Possible WG: caching

Comments

@Gregable
Copy link
Member

Gregable commented Sep 20, 2018

Current behavior

The AMP Validator currently emits validation parse errors on malformed CSS in custom stylesheets (<style amp-custom> and <style amp-keyframes>). For example, publishers may see an error with attached line/column information such as:

CSS syntax error in tag 'style amp-custom' - invalid declaration.

The validator does not currently parse @media query strings, for example, the string screen and (max-width: 500px) within the CSS block @media screen and (max-width: 500px) {}.

It also does not make any assertions on the valid values for media query types (ex. screen) or media query features (ex. max-width).

Intended change

This proposed change is to invalidate documents containing parse errors in @media query strings, and validate media types and features against a whitelist of allowed values, depending on the context. See the below sections for additional detail.

Parse Errors

If there are errors generated while parsing CSS media queries, browsers will currently discard the entire encountered @media rule. The AMP Validator ignores these cases, but will begin to detect and report on them. The generated validator messages will look like:

CSS syntax error in tag 'style amp-custom' - malformed media query.

And will attach line / column information referencing the start of the media query.

Examples of invalid media queries include:

  • screen and(max-width:500px)
  • screen and
  • screen,
  • screen (
  • ((min-width:500px))
  • &test, screen
  • not only screen and (color)

A sample of 10,000 crawler-accessible pages shows 11 amp documents (0.11%) that would become invalid if media query parsing would report a validator error.

Whitelist of valid media query types and features

The set of media query types (ex. screen) and media query features (ex. max-width) will be whitelisted by the validator.

For normal AMP documents, it is intended that all types and features with any significant use will be whitelisted, so only uncommon typos will become invalid. The current list, which will be extended after wider testing includes:

media_query_type: "all"
media_query_type: "print"
media_query_type: "screen"
media_query_type: "speech"
media_query_type: "tty"
media_query_type: "tv"
media_query_type: "projection"
media_query_type: "handheld"
media_query_type: "braille"
media_query_type: "embossed"
media_query_type: "aural"
media_query_feature: "width"
media_query_feature: "height"
media_query_feature: "aspect-ratio"
media_query_feature: "orientation"
media_query_feature: "resolution"
media_query_feature: "scan"
media_query_feature: "grid"
media_query_feature: "update"
media_query_feature: "overflow-block"
media_query_feature: "overflow-inline"
media_query_feature: "color"
media_query_feature: "color-gamut"
media_query_feature: "color-index"
media_query_feature: "display-mode"
media_query_feature: "monochrome"
media_query_feature: "inverted-colors"
media_query_feature: "pointer"
media_query_feature: "hover"
media_query_feature: "any-pointer"
media_query_feature: "any-hover"
media_query_feature: "light-level"
media_query_feature: "prefers-reduced-motion"
media_query_feature: "prefers-reduced-transparency"
media_query_feature: "prefers-contrast"
media_query_feature: "prefers-color-scheme"
media_query_feature: "scripting"
media_query_feature: "device-width"
media_query_feature: "device-height"
media_query_feature: "device-aspect-ratio"
# Other non-standard media types and features commonly seen:
media_query_type: "-sass-debug-info"
media_query_type: "device-pixel-ratio"
media_query_type: "device-pixel-ratio2"
media_query_feature: "device-pixel-ratio"
media_query_feature: "device-pixel-ratio2"
media_query_feature: "high-contrast"
media_query_feature: "--mod"
media_query_feature: "--md"
media_query_feature: "aspect-ratio"

This list assumes that min-, max- prefix variations, as well as vendor prefix variations (-o-, -moz, -ms-, -webkit-) of the above will also be considered valid.

The validation messages will look like:

CSS syntax error in tag 'style amp-custom' - disallowed media type 'foo'.
CSS syntax error in tag 'style amp-custom' - disallowed media feature 'foo'.

For AMP documents, we will whitelist any commonly observed type or feature, so the impact should be very small and limited to rare typos.

For other types of AMP documents, such as AMP4ADS or AMP4EMAIL, the whitelist will be constrained to a smaller subset.

AMP4ADS:

type: 'all'
type: 'print'
type: 'screen'
type: 'speech'
feature: 'width'
feature: 'height'
feature: 'aspect-ratio'
feature: 'orientation'
feature: 'resolution'

AMP4EMAIL: TBD

Purpose of Change
Some media query types and features can be used to fingerprint user browsers representing a security and privacy risk in some contexts. Adding this functionality to manage access to media query parameters allows AMP providers to manage this risk.

Suggested Change Schedule
Upon approval of this change, the AMP validator will begin emitting warnings for documents that contain unparseable media queries or use media query types or features not on the proposed whitelist. These warnings will not cause AMP documents to become invalid, but will give developers time to understand the impact of the change. Warnings will appear in all AMP validator interfaces, including Google Search Console.

No sooner than 6 weeks after warnings have been emitted, these warnings will become errors, and documents with these warnings will become invalid.

For process reference, see spec/amp-versioning-policy.md.

@Gregable Gregable added Category: Validator INTENT TO IMPLEMENT Proposes implementation of a significant new feature. https://bit.ly/amp-contribute-code labels Sep 20, 2018
@dreamofabear
Copy link

Which media query types/features will no longer be valid?

@Gregable
Copy link
Member Author

Anything not in the whitelist. However, any type/feature we find used with any significance on the AMP corpus will be added to the AMP whitelist. Some of the ones in that list fall in that category, such as "-sass-debug-info".

@aghassemi
Copy link
Contributor

few older types to add: tty, tv, projection, handheld, braille, embossed, aural. otherwise LGTM

@Gregable
Copy link
Member Author

@aghassemi I updated the first post to add those 7 to the list.

@dreamofabear
Copy link

LGTM

@ampprojectbot ampprojectbot added this to the Pending Triage milestone Sep 25, 2018
@cramforce
Copy link
Member

LGTM

@mrjoro
Copy link
Member

mrjoro commented Jan 15, 2019

Is this I2I still active?

@Gregable
Copy link
Member Author

Yes. However, I think it's still in need of a third LGTM to make progress?

@mrjoro
Copy link
Member

mrjoro commented Jan 16, 2019

We could probably consider this one grandparented in, but it doesn't hurt. :)

cc @ampproject/wg-approvers for a third pre-approval LGTM

@aghassemi
Copy link
Contributor

3rd LGTM

@Gregable
Copy link
Member Author

This has been implemented as a warning only. I don't anticipate we'll want to move to an error for AMP, only email, so closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
INTENT TO IMPLEMENT Proposes implementation of a significant new feature. https://bit.ly/amp-contribute-code P3: When Possible WG: caching
Projects
None yet
Development

No branches or pull requests

6 participants