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

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

Closed
DK-Stern opened this issue Feb 18, 2022 · 0 comments

Comments

@DK-Stern
Copy link
Contributor

DK-Stern commented Feb 18, 2022

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:

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.

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

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:

} else {
this.transformedNodesCounter--;
}

PR

For the first solution i've created a PR: #1303

@DK-Stern DK-Stern changed the title SSR: Duplicated ids on nodes with attributes 'media', 'heights' and 'sizes' SSR: Duplicated ids on nodes with attributes 'media', 'heights' or 'sizes' Feb 18, 2022
@DK-Stern DK-Stern changed the title SSR: Duplicated ids on nodes with attributes 'media', 'heights' or 'sizes' SSR: Duplicated ids if there are nodes with attributes 'media', 'heights' or 'sizes' Feb 18, 2022
@DK-Stern DK-Stern changed the title SSR: Duplicated ids if there are nodes with attributes 'media', 'heights' or 'sizes' SSR: Duplicated ids in rendered markup, if there are nodes with attributes 'media', 'heights' or 'sizes' Feb 18, 2022
sebastianbenz pushed a commit that referenced this issue Mar 1, 2022
…e are nodes with attributes 'media', 'heights' or 'sizes'" (#1303)

* fix duplicated ids

* Added transformation test and refactored fix.

* Adjusted input.html of transformation test with more realistic example
sebastianbenz pushed a commit that referenced this issue Mar 10, 2022
…e are nodes with attributes 'media', 'heights' or 'sizes'" (#1303)

* fix duplicated ids

* Added transformation test and refactored fix.

* Adjusted input.html of transformation test with more realistic example
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants