SSR: Duplicated ids in rendered markup, if there are nodes with attributes 'media', 'heights' or 'sizes'

[DK-Stern]

Issue

On SSR generated amp pages are some nodes with same id, which breaks the logic of generated css rules with display: block, so some images wouldn't be displayed.

Reason

Because of decreasing the counter transformedNodesCounter on following line, we've got on some nodes duplicated ids generated:

https://github.com/ampproject/amp-toolbox/blob/b367a2db78e1934e5c3e72727777e7c29d6a3b35/packages/optimizer/lib/transformers/ApplyCommonAttributes.js#L220

I cannot say why this happen, but we can ensure an easy fix to preventing of generating duplicated ids.

Solutions

First possible solution

One possible solution would be, to add the generated id to the Set of ids before line 269 to find collisions.

https://github.com/ampproject/amp-toolbox/blob/b367a2db78e1934e5c3e72727777e7c29d6a3b35/packages/optimizer/lib/transformers/ApplyCommonAttributes.js#L265-L270

That would look like this:

[...]
    if (this.ids.has(id)) {
      // generate a new id if this one already exists
      return this.getOrCreateId(node);
    }
    this.ids.add(id);
    return id;
  }

Second possible solution

A other possible solution would be, to remove the else condition where the counter transformedNodesCounter will be decreased:

https://github.com/ampproject/amp-toolbox/blob/b367a2db78e1934e5c3e72727777e7c29d6a3b35/packages/optimizer/lib/transformers/ApplyCommonAttributes.js#L219-L221

PR

For the first solution i've created a PR: https://github.com/ampproject/amp-toolbox/pull/1303