Skip to content

Bump amp-custom style size limit from 50000 to 75000 bytes#4313

Merged
westonruter merged 1 commit intodevelopfrom
update/stylesheet-limit
Feb 18, 2020
Merged

Bump amp-custom style size limit from 50000 to 75000 bytes#4313
westonruter merged 1 commit intodevelopfrom
update/stylesheet-limit

Conversation

@westonruter
Copy link
Copy Markdown
Member

@westonruter westonruter commented Feb 18, 2020

Summary

See ampproject/amphtml#26466 and ampproject/amphtml#26475.

This PR directly patches class-amp-allowed-tags-generated.php without updating the entire spec which will include a lot of other changes. This will facilitate a little 1.4.4 release with a big impact.

The validator update has been deployed:

AMP Validator showing 75000 byte size limit for amp-custom styles

Checklist

  • My pull request is addressing an open issue (please create one otherwise).
  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@googlebot googlebot added the cla: yes Signed the Google CLA label Feb 18, 2020
@westonruter westonruter added this to the v1.4.4 milestone Feb 18, 2020
@westonruter westonruter force-pushed the update/stylesheet-limit branch from 22cfca1 to 90ff57a Compare February 18, 2020 19:45
@westonruter westonruter requested a review from amedina February 18, 2020 20:23
Copy link
Copy Markdown
Member

@amedina amedina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! Excellent to see this land.

@westonruter westonruter merged commit 542dee8 into develop Feb 18, 2020
@westonruter westonruter deleted the update/stylesheet-limit branch February 18, 2020 21:51
westonruter added a commit that referenced this pull request Feb 19, 2020
* tag '1.4.4':
  Bump 1.4.4
  Simplify notice text for Stories removal (#4314)
  Bump amp-custom style limit from 50000 to 75000 bytes (#4313)
  Use travis_retry on PHPUnit for external-http tests (#4298)
  Bump version to 1.4.4-alpha
  Bump 1.4.3
sprintf(
/* translators: 1: Documentation URL, 2: Documentation URL, 3: !important */
__( 'AMP allows you to <a href="%1$s">style your pages using CSS</a> in much the same way as regular HTML pages, however there are some <a href="%2$s">restrictions</a>. Nevertheless, the AMP plugin automatically inlines external stylesheets, transforms %3$s qualifiers, and uses tree shaking to remove the majority of CSS rules that do not apply to the current page. Nevertheless, AMP does have a 50KB limit and tree shaking cannot always reduce the amount of CSS under this limit; when this happens an excessive CSS error will result.', 'amp' ),
__( 'AMP allows you to <a href="%1$s">style your pages using CSS</a> in much the same way as regular HTML pages, however there are some <a href="%2$s">restrictions</a>. Nevertheless, the AMP plugin automatically inlines external stylesheets, transforms %3$s qualifiers, and uses tree shaking to remove the majority of CSS rules that do not apply to the current page. Nevertheless, AMP does have a 75KB limit and tree shaking cannot always reduce the amount of CSS under this limit; when this happens an excessive CSS error will result.', 'amp' ),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@westonruter Using a placeholder for the limit here would make the string more future-proof, just an idea.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I thought about that. However, it is extremely unlikely the limit will be changed again. So that's why I didn't bother.

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

Labels

cla: yes Signed the Google CLA

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants