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

Meta tags with same name attribute but different other attributes will be overwritten #34

Closed
GoodbyeNJN opened this issue Sep 6, 2023 · 3 comments · Fixed by #35
Closed

Comments

@GoodbyeNJN
Copy link
Contributor

For example, use the following two tags:

<Meta name="theme-color" media="(prefers-color-scheme: light)" content="#fff" />
<Meta name="theme-color" media="(prefers-color-scheme: dark)" content="#000" />

Only the latter will actually be rendered.

The problem lies here:

solid-meta/src/index.tsx

Lines 156 to 170 in c9d21ef

actions.addServerTag = (tagDesc: TagDescription) => {
const { tags = [] } = props;
// tweak only cascading tags
if (cascadingTags.indexOf(tagDesc.tag) !== -1) {
const index = tags.findIndex(prev => {
const prevName = prev.props.name || prev.props.property;
const nextName = tagDesc.props.name || tagDesc.props.property;
return prev.tag === tagDesc.tag && prevName === nextName;
});
if (index !== -1) {
tags.splice(index, 1);
}
}
tags.push(tagDesc);
};

This function seems to remove duplicate tags, but only name or property attributes are compared when checking whether they are duplicates.

I suppose to sort props by key and then serialize it as the unique key of a tag for comparison.

@DaniGuardiola
Copy link

This fix broke my website's open graph meta tags.

I had a top level set of meta tags with default, e.g.

<Meta name="og:image" property="og:image" content="default.png" />

And I used child meta tags to override it, e.g.

<Meta name="og:image" property="og:image" content="actual-image.png" />

It worked fine. However, due to this change, it now doesn't override the tag, and both are shown at the same time, causing tools reading the open graph tags to use the first one and not the correct one.

This is because the key now is different for both:

// first key
'meta{"content":"default.png","name":"og:image"}'

// second key
'meta{"content":"actual-image.png","name":"og:image"}'

@GoodbyeNJN
Copy link
Contributor Author

@DaniGuardiola Sorry for breaking existing features while fixing this issue.
I'm reorganizing the attributes of meta element and plan to list out a separate test case for meta element first.
Once a separate test case for meta element is approved by the maintainer, I would re-fix this issue.

@DaniGuardiola
Copy link

@GoodbyeNJN it's okay, shit happens :)

See #39 - I think the cascading/overriding effect should just be completely explicit and opt-in because the heuristics get too complicated otherwise.

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

Successfully merging a pull request may close this issue.

2 participants