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

Accelerated Mobile Pages (AMP) + HTML Compressor #695

Closed
raamdev opened this Issue Feb 28, 2016 · 10 comments

Comments

Projects
None yet
5 participants
@raamdev
Contributor

raamdev commented Feb 28, 2016

The Comet Cache HTML Compressor currently causes several validation errors when applied to AMP pages (which are loaded by appending /amp/ to the URL of a page).

AMP has very specific rules for the markup of a page, as the whole point of the AMP project is to create somewhat of a standard for maximizing the speed and simplicity of displaying pages on mobile devices.

The following two sections particularly relevant to the HTML Compressor are copied from the AMP HTML Format Spec:


Required markup

AMP HTML documents MUST

  • start with the doctype <!doctype html>.
  • contain a top-level <html ⚡> tag (<html amp> is accepted as well).
  • contain <head> and <body> tags (They are optional in HTML).
  • contain a <link rel="canonical" href="$SOME_URL" /> tag inside their head that points to the regular HTML version of the AMP HTML document or to itself if no such HTML version exists.
  • contain a <meta charset="utf-8"> tag as the first child of their head tag.
  • contain a <meta name="viewport" content="width=device-width,minimum-scale=1"> tag inside their head tag. It's also recommend to include initial-scale=1 (1).
  • contain a <script async src="https://cdn.ampproject.org/v0.js"></script> tag inside their head tag.
  • contain the AMP boilerplate code in their head tag.

(1) width=device-width,minimum-scale=1 is required to ensure GPU rasterization is enabled.

Custom fonts

Authors may include stylesheets for custom fonts. The 2 supported methods are link tags pointing to whitelisted font providers and @font-face inclusion.

Example:

<link rel="stylesheet" href="https://fonts.googleapis.com/css?family=Tangerine">

Font providers can be whitelisted if they support CSS-only integrations and serve over HTTPS. The following origins are currently allowed for font serving via link tags:

IMPLEMENTERS NOTE: Adding to this list requires a change to the Google AMP Cache CSP rule.

Authors are free to include all custom fonts via a @font-face CSS instruction via their custom CSS. Fonts included via @font-face must be fetched via the HTTP or HTTPS scheme.


Validation Errors Caused by the HTML Compressor

Given all these requirements, it doesn't look like the HTML Compressor will be able to do any combining of resources into smaller files, as that violates several of these rules and throws validations errors.

Here's an example of those validation errors when the HTML Compressor is enabled (the blurred out errors are not related to the HTML Compressor or Comet Cache):

2016-02-28_15-05-04


How to Improve Compatibility

The easiest way to improve compatibility with AMP would be to always disable specific HTML Compressor options when compressing /amp/ pages. That way AMP pages can still benefit from things like HTML Compression.

In my testing, the following HTML Compressor configuration works fine with AMP pages. If we can turn off the highlighted options below whenever compressing an /amp/ page, that will be a great way to improve compatibility:

2016-02-28_15-01-11

@Li-An

This comment has been minimized.

Li-An commented May 26, 2016

I confirm the problem with AMP and last version of Comet Cache. I do not use HTML compression on my sites and I've got a bunch of 404 errors. See here

@raamdev

This comment has been minimized.

Contributor

raamdev commented May 26, 2016

@Li-An This GitHub issue is specifically related to Comet Cache + HTML Compressor and the AMP plugin. If you're not using the HTML Compressor and you're seeing an issue, please provide us with some additional details about the problem so that we can investigate more closely.

Also, the URLs in your screenshot indicate the 404 errors were for blog/11/, etc. However, the WordPress AMP plugin specifically loads pages only when /amp/ is appended to the URL, so it doesn't even look like those URLs (blog/11/, etc.) are using AMP.

@Li-An

This comment has been minimized.

Li-An commented May 26, 2016

OK, sorry. After reflexion I may be wrong but cannot understant where the problem comes from.

@e11productions

This comment has been minimized.

e11productions commented Oct 12, 2016

After trying the suggestions for HTML compression setting, the page speed dropped significantly. Will a future update correct the issue? If not, are there any other suggestions you may have?

@raamdev

This comment has been minimized.

Contributor

raamdev commented Oct 13, 2016

@e11productions As described above under "Validation Errors Caused by the HTML Compressor", the markup required by AMP (see "Required markup" above) does not support compression. If you try to compress AMP markup, you will get validation errors. There's nothing Comet Cache can do about that, as we have no control over what AMP requires.

@raamdev

This comment has been minimized.

Contributor

raamdev commented Oct 13, 2016

@e11productions writes...

Will a future update correct the issue?

As described earlier in this issue, a future update will selectively disable the appropriate HTML Compressor features only when an AMP page is being loaded, yes. Right now, disabling some of the HTML Compressor features as described above (so that you don't get AMP validation errors) also disables those HTML Compressor features for non-AMP pages, which is of course not ideal.

The fix we'll be implementing will selectively disable those HTML Compressor features for AMP pages, but there's no way to make all of the HTML Compressor features work with AMP pages due to the requirements set forth in the AMP HTML Format Spec.

jaswrks pushed a commit to websharks/html-compressor that referenced this issue Dec 7, 2016

jaswsinc
- Adding new config option: `amp_exclusions_enable` (default is enabl…
…ed). This improves compatibility with [Accelerated Mobile Pages](https://www.ampproject.org/). When this option is enabled and the URI being compressed ends with `/amp/`, or the document contains a top-level `<html ⚡>` tag (`<html amp>` is accepted as well), then features which are incompatible with [Accelerated Mobile Pages](https://www.ampproject.org/) will be disabled accordingly, regardless of your other settings. See [Issue #695](websharks/comet-cache#695) in the Comet Cache repo.

- Adding `isDocAmpd()` conditional check against current URI & document.
- Adding automatic AMP feature exclusions for improved AMP compatibility.
- Optimizing for speed by removing unnecessary calls to `unset()`.
- Enhancing unicode compatibility by taking full advantage of all `mb_*()` functions.
- Adding multibyte compatible `pregQuote()`.
- Adding multibyte compatible `replaceOnce()`.
- Adding multibyte compatible `substrReplace()`.
- Updating all regex patterns to add the `/u` flag for unicode compatibility.
- Updating minimum PHP requirement. Now requires PHP v5.4+ in support of short array syntax `[]`.

@jaswrks jaswrks referenced this issue Dec 7, 2016

Merged

PR: feature/695 #80

jaswrks pushed a commit to websharks/html-compressor that referenced this issue Dec 7, 2016

jaswsinc

jaswrks pushed a commit to websharks/comet-cache-pro that referenced this issue Dec 7, 2016

jaswsinc

jaswrks pushed a commit to websharks/comet-cache-pro that referenced this issue Dec 7, 2016

@jaswrks

This comment has been minimized.

Member

jaswrks commented Dec 8, 2016

Noting that a follow-up release of the HTML Compressor was needed here as well, due to the bug mentioned in the release notes: https://github.com/websharks/html-compressor/releases/tag/161208

jaswrks pushed a commit to websharks/comet-cache-pro that referenced this issue Dec 8, 2016

jaswsinc
@renzms

This comment has been minimized.

renzms commented Dec 16, 2016

@raamdev @jaswsinc

Confirmed Working

This is confirmed working for <html ⚡>, <html amp> and URLS ending in /amp.

Page loads normally without console errors while the HTML Compressor is active with the option to detect AMP + disable incompatible features.

screen shot 2016-12-16 at 8 06 49 am

I aslo tested this using the AMP Plugin

Please note that the AMP Plugin hasn't been updated for at least 2 months and has not been tested with WP 4.7 so there are some features that were incompatible with this version of WordPress.

screen shot 2016-12-16 at 8 09 12 am

Deprecated: Non-static method AMP_Customizer_Design_Settings::register_customizer_ui() should not be called statically in /app/src/wp-includes/class-wp-hook.php on line 298 

Call Stack: 0.0009 388392 
1. {main}() /app/src/index.php:0 0.0017 388736 
2. require('/app/src/wp-blog-header.php') /app/src/index.php:17 0.2673 3986248 
3. require_once('/app/src/wp-includes/template-loader.php') /app/src/wp-blog-header.php:19 0.2674 3986248 
4. do_action() /app/src/wp-includes/template-loader.php:12 0.2674 3986624 
5. WP_Hook->do_action() /app/src/wp-includes/plugin.php:453 0.2674 3986624 
6. WP_Hook->apply_filters() /app/src/wp-includes/class-wp-hook.php:323 0.2699 3987800 
7. amp_render() /app/src/wp-includes/class-wp-hook.php:298 0.3015 4080504 
8. do_action() /app/src/wp-content/plugins/amp/amp.php:127 0.3015 4080880 
9. WP_Hook->do_action() /app/src/wp-includes/plugin.php:453 0.3015 4080880 
10. WP_Hook->apply_filters() /app/src/wp-includes/class-wp-hook.php:323 0.3015 4082008 
11. AMP_Template_Customizer->init_preview() /app/src/wp-includes/class-wp-hook.php:298 0.3015 4082008 
12. AMP_Template_Customizer->register_ui() /app/src/wp-content/plugins/amp/includes/admin/class-amp-customizer.php:99 0.3018 4086360 
13. do_action() /app/src/wp-content/plugins/amp/includes/admin/class-amp-customizer.php:117 0.3018 4086736 
14. WP_Hook->do_action() /app/src/wp-includes/plugin.php:453 0.3018 4086736 
15. WP_Hook->apply_filters() /app/src/wp-includes/class-wp-hook.php:323

raamdev added a commit that referenced this issue Dec 21, 2016

Phing release of v161221 with the following changes:
- **Bug Fix:** Improving PHP OPcache detection. Now considering the INI option `opcache.restrict_api`. Comet Cache is now smart enough to avoid generating the PHP Warning: _PHP Warning: Zend OPcache API is restricted by "restrict_api" configuration directive_. See [Issue #733](#733).
- **New Feature (Pro): Mobile Mode.** This release adds a new feature that is designed to improve compatibility with Adaptive themes for mobile devices. To learn more, please see: **Dashboard → Comet Cache Pro → Plugin Options → Mobile Mode**. See also: [Issue #471](#471).
- **Enhancement: Auto-Clearing Author Page Cache.** This release makes Comet Cache smart enough to detect when a user is deleted (or removed from a child blog in a Network), at which time the Author page for that user will be cleared from the cache so it can be regenerated automatically. See [Issue #304](#304).
- **Enhancement: Multibyte Compatibility.** This release improves support for WordPress Permalinks that contain UTF-8 symbols (or emojis) in them. More specifically, this release adds the `/u` flag to all `preg_*()` calls in cache clearing routines that generate cache paths from Watered-Down Regex patterns entered by a site owner. See: [Issue #611](#611).
- **Enhancement: Widget Change Detection.** Comet Cache can now detect when **Appearance → Widgets** are added/edited/removed, and Comet Cache will automatically clear the cache so that your site remains up-to-date. See [Issue #411](#411).
- **Enhancement (Pro): Static CDN Filters and `srcset`.** This release enhances Static CDN Filters in Comet Cache Pro. Static CDN Filters are now smart enough to filter all image sources included in an `srcset=""` attribute that is generated by WordPress. See [Issue #660](#660). If you'd like to learn more about `srcset=""`, see [this article at WordPress.org](https://make.wordpress.org/core/2015/11/10/responsive-images-in-wordpress-4-4/).
- **Enhancement (Pro): Automatic Background Updates.** It is now possible to enable automatic background updates that occur quietly in the background whenever new features, bug fixes, or security issues are addressed by our developers. See: **Dashboard → Comet Cache Pro → Config. Options → Update Credentials**. See also: [Issue #827](#827).
- **Enhancement (Pro): HTML Compressor + Accelerated Mobile Pages (AMP).** Updated to the latest available release of the HTML Compressor (v161208) with improved support for [Accelerated Mobile Pages](https://www.ampproject.org/). See: [Issue #695](#695). See also: [HTML Compressor v161208 changelog](https://github.com/websharks/html-compressor/releases/tag/161208).
- **Enhancement (Pro): HTML Compressor / AMP Compatibility.** Improved compatibility with [Accelerated Mobile Pages](https://www.ampproject.org/). There is a new HTML Compressor option that is enabled by default and it makes Comet Cache smart enough to auto-detect and selectively disable portions of the HTML Compressor that are incompatible with the AMP spec; i.e., routines that are not necessary when serving APMd pages. In short, if the URI being compressed ends with `/amp/`, or the document contains a top-level `<html ⚡>` tag (`<html amp>` is accepted as well), then features which are incompatible with [Accelerated Mobile Pages](https://www.ampproject.org/) will be disabled accordingly.
- **Compatibility:** Avoid deprecated `wp_get_sites()` and use `get_sites()` instead. See [Issue #848](#848).
- **Documentation:** Added Watered-Down Regex documentation notes to the inline documentation (in the software) about the use of `^` and `$` in some places where these special characters are not fully supported. Also adding the same notes to the [Watered-Down Regex KB Article](https://cometcache.com/r/watered-down-regex-syntax/). See also: [Issue #611](#611).
@raamdev

This comment has been minimized.

Contributor

raamdev commented Dec 21, 2016

Comet Cache v161221 has been released and includes changes from this GitHub Issue. See the v161221 announcement for further details.


This issue will now be locked to further updates. If you have something to add related to this GitHub Issue, please open a new GitHub Issue and reference this one (#695).

@raamdev raamdev closed this Dec 21, 2016

@websharks websharks locked and limited conversation to collaborators Dec 21, 2016

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