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 min--moz-device-pixel-ratio and max--moz-device-pixel-ratio media features to CSS validator spec #34553

Conversation

westonruter
Copy link
Member

@westonruter westonruter commented May 26, 2021

In the past few days the WordPress AMP plugin has been getting topics about Search Console reporting a warning for a CSS syntax error:

  1. https://wordpress.org/support/topic/amp-css-syntax-error-in-tag-style-amp-custom-2/
  2. https://wordpress.org/support/topic/css-syntax-error-in-tag-style-amp-custom-11/
  3. https://wordpress.org/support/topic/css-syntax-error-in-tag-style-amp-custom-12/
  4. https://wordpress.org/support/topic/css-syntax-error-tag-style-amp-custom-disallowed-min-moz-device-pixel-ratio-2/
  5. CSS syntax error in tag 'style amp-custom'. error amp-wp#6386

For example:

search-console

The offending CSS code looks like this:

@media only screen and (-webkit-min-device-pixel-ratio: 1.5),
       only screen and (min--moz-device-pixel-ratio: 1.5), 
       only screen and (min-resolution: 240dpi) 
{
	div.googleplay a.apptitle {
		background: url(../img/googleplay-small.png);
	}
}

Both the JS and C++ validators are reporting:

CSS syntax error in tag 'style amp-custom' - disallowed media feature 'min--moz-device-pixel-ratio'

The fix appears to simply be to add both min--moz-device-pixel-ratio and max--moz-device-pixel-ratio to the list of non-standard media types and features commonly seen. These two features were used in Firefox 16 and older according to MDN.

@amp-owners-bot amp-owners-bot bot requested a review from Gregable May 26, 2021 04:02
@amp-owners-bot
Copy link

amp-owners-bot bot commented May 26, 2021

Hey @ampproject/wg-caching! These files were changed:

validator/validator-css.protoascii

@Gregable Gregable merged commit 06d6274 into ampproject:main Jun 1, 2021
@westonruter
Copy link
Member Author

@Gregable Will this change be live for Search Console in the next week or so?

@Gregable
Copy link
Member

Gregable commented Jun 7, 2021

I would guess yes. @honeybadgerdontcare is the current release owner.

@honeybadgerdontcare
Copy link
Contributor

@westonruter this appears to be in prod for both internal and external validator interfaces.

@westonruter
Copy link
Member Author

I don't see it live when testing this URL: https://wrestling.sportsjagran.com/wwe/3-wwe-superstars-who-may-join-aew/?amp

I'm checking via https://search.google.com/test/amp and also https://playground.amp.dev/

In both cases I still see:

CSS syntax error in tag 'style amp-custom' - disallowed media feature 'min--moz-device-pixel-ratio'.

@honeybadgerdontcare
Copy link
Contributor

honeybadgerdontcare commented Jun 7, 2021

I was testing with the code snippet in comment #1 and adding <style amp-custom>.

<style amp-custom>
@media only screen and (-webkit-min-device-pixel-ratio: 1.5),
       only screen and (min--moz-device-pixel-ratio: 1.5), 
       only screen and (min-resolution: 240dpi) {
	div.googleplay a.apptitle {
		background: url(../img/googleplay-small.png);
	}
}
</style>

validator.amp.dev shows both that code snippet and that URL pass. Also our internal tool also shows it passing (go/ampvalidator).

Update: I see the issue was to remove the warning. The internal tool does still return the warning.

@honeybadgerdontcare
Copy link
Contributor

Another release is happening tomorrow, I'll verify then that the warning has been removed (apologies, thought it was an error being removed).

@honeybadgerdontcare
Copy link
Contributor

@sebastianbenz this change is now live. However it still reports this as a warning. I've confirmed this is true for both internal and external. You can confirm by adding a test file for it and updating the corresponding .out file. I'm unsure of why it's not accepting it and I haven't looked at the css validation code in a long time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants