Amp-wp: Document parsing fails when HTML start tag contains ⚡

Created on 26 Oct 2020 · 8Comments · Source: ampproject/amp-wp

Hi there,

we just started using the AMP Optimizer library and found a bug. In our admittedly strange setup, we use a class binding on the html element to set some marker classes. These class binding gets removed when using the AMP Optimizer.

Input:

<!doctype html>
<html ⚡ [class]="mystate.class" class="blue">
<head>
    <meta charset="utf-8">
    <title>My AMP Page</title>
    <link rel="canonical" href="self.html" />
    <meta name="viewport" content="width=device-width,minimum-scale=1,initial-scale=1">
    <style amp-boilerplate>body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style><noscript><style amp-boilerplate>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>
    <script async src="https://cdn.ampproject.org/v0.js"></script>
    <script async custom-element="amp-bind" src="https://cdn.ampproject.org/v0/amp-bind-0.1.js"></script>
    <style amp-custom>
        h1 {
            margin: 1rem;
        }
        .blue {
            background-color: blue;
        }
        .yellow {
            background-color: yellow;
        }
    </style>
</head>
<body>
<amp-state id="mystate">
    <script type="application/json">
        {
            "class": "blue"
        }
    </script>
</amp-state>
<button on="tap:AMP.setState({mystate: {class: 'yellow'}})">
    yellow
</button>
<button on="tap:AMP.setState({mystate: {class: 'blue'}})">
    blue
</button>
</body>
</html>

Output (the relevant part):

<!DOCTYPE html>
<html ⚡ class="blue" i-amphtml-layout="" i-amphtml-no-boilerplate="" transformed="self;v=1">
<head><meta charset="utf-8">

So after the optimization, clicking on the buttons has no effect.

We use the Optimizer with the default configuration (nothing special).

Bug Core

Source

dritter

All 8 comments

@dritter Are you using the Optimizer without the rest of the WordPress AMP logic?

How are you passing the HTML into the Optimizer for processing?

westonruter on 26 Oct 2020

Humm, I can reproduce this problem as well, just by using this to start an AMP page:

<html ⚡ [class]="mystate.class" class="blue">

A quick fix looks to be to just use the amp attribute instead of ⚡︎:

<html amp [class]="mystate.class" class="blue">

I then get the expected:

<html amp data-amp-bind-class="mystate.class" class="blue" i-amphtml-layout="" i-amphtml-no-boilerplate="" transformed="self;v=1">

westonruter on 26 Oct 2020

👍1

Are you using the Optimizer without the rest of the WordPress AMP logic?

Yep.

How are you passing the HTML into the Optimizer for processing?

We implemented a Symfony EventSubscriber for that purpose:

<?php

namespace App\EventSubscriber;

use AmpProject\Optimizer\ErrorCollection;
use AmpProject\Optimizer\TransformationEngine;
use Psr\Log\LoggerInterface;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpKernel\Event\ResponseEvent;
use Symfony\Component\HttpKernel\KernelEvents;

class AmpOptimizerSubscriber implements EventSubscriberInterface
{
    /** @var TransformationEngine */
    private $transformationEngine;
    /** @var LoggerInterface */
    private $logger;

    /**
     * AmpOptimizerSubscriber constructor.
     * @param TransformationEngine $transformationEngine
     * @param LoggerInterface $logger
     */
    public function __construct(
        TransformationEngine $transformationEngine,
        LoggerInterface $logger
    ) {
        $this->transformationEngine = $transformationEngine;
        $this->logger = $logger;
    }

    /** @return array */
    public static function getSubscribedEvents(): array
    {
        return [
            KernelEvents::RESPONSE => 'onKernelResponse',
        ];
    }

    /**
     * @param ResponseEvent $event
     */
    public function onKernelResponse(ResponseEvent $event): void
    {
        if (!$event->isMasterRequest()) {
            return;
        }
        $response = $event->getResponse();
        $errorCollection = new ErrorCollection();

        $optimizedHtml = $this->transformationEngine->optimizeHtml($response->getContent(), $errorCollection);

        if ($errorCollection->count() > 0) {
            foreach ($errorCollection as $error) {
                $this->logger->error(sprintf(
                    "AMP-Optimizer Error code: %s\nError Message: %s\n",
                    $error->getCode(),
                    $error->getMessage()
                ));
            }
        }

        $optimizedHtml .= "\n<!-- Optimized with AMP Optimizer -->";
        $response->setContent($optimizedHtml);
    }
}

This has some of Symfonys DI magic in here, but the injected TransformationEngine will be instantiated with new (without parameters) somewhere.

dritter on 26 Oct 2020

This doesn't appear to have anything to do with the Optimizer codebase, but rather an issue with the \AmpProject\Dom\Document class. But you can work around this issue by using the amp attribute for now.

westonruter on 26 Oct 2020

Awesome. Thanks for the quick response! :tada:

dritter on 26 Oct 2020

While it's awesome to see the Optimizer library already being used outside of the AMP plugin, it's also quite scary given it hasn't been officially published as a separate library yet (but we're working on it!).

The transformation of the bind parameters into their data-amp-bind-* form has a fallback to the untransformed attribute in case a problem occurred with the regex to match them. I assume the emoji is throwing one of our regexes off here... I'll investigate.

schlessera on 26 Oct 2020

👍1

While it's awesome to see the Optimizer library already being used outside of the AMP plugin, it's also quite scary given it hasn't been officially published as a separate library yet (but we're working on it!).

YaY! Well, "started using" is a bit too much to say. We "are evaluating it" fits better ;)

dritter on 26 Oct 2020

@dritter You might want to keep an eye on https://github.com/ampproject/amp-wp/issues/4567 to see the progress on extracting it into a separate package you can pull in via Composer.

schlessera on 26 Oct 2020

👍1

Was this page helpful?

0 / 5 - 0 ratings