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

Do not make TL_HEAD tags unique #1214

Closed
qzminski opened this issue Dec 2, 2017 · 6 comments
Closed

Do not make TL_HEAD tags unique #1214

qzminski opened this issue Dec 2, 2017 · 6 comments

Comments

@qzminski
Copy link
Member

qzminski commented Dec 2, 2017

The TL_HEAD tags that are being added to the page are first filtered out of duplicates. This is could be a problem for OpenGraph tags where you can define the arrays – http://ogp.me/#array.

https://github.com/contao/core-bundle/blob/master/src/Resources/contao/library/Contao/Controller.php#L898

So if you would have two images and if their dimensions would be the same the output code should look like this:

<meta property="og:image" content="http://example.com/rock.jpg" />
<meta property="og:image:width" content="300" />
<meta property="og:image:height" content="300" />
<meta property="og:image" content="http://example.com/rock2.jpg" />
<meta property="og:image:width" content="300" />
<meta property="og:image:height" content="300" />

But because the array_unique is called, some of the image size tags would be stripped:

$GLOBALS['TL_HEAD'][] = '<meta property="og:image" content="http://example.com/rock.jpg" />';
$GLOBALS['TL_HEAD'][] = '<meta property="og:image:width" content="300" />';
$GLOBALS['TL_HEAD'][] = '<meta property="og:image:height" content="300" />';
$GLOBALS['TL_HEAD'][] = '<meta property="og:image" content="http://example.com/rock2.jpg" />';
$GLOBALS['TL_HEAD'][] = '<meta property="og:image:width" content="300" />'; # will be dropped
$GLOBALS['TL_HEAD'][] = '<meta property="og:image:height" content="300" />'; # will be dropped

Contao 3 is affected as well. Also see codefog/contao-social_images#19

@backbone87
Copy link
Contributor

Imho you should add all 3 tags for one Image into one TL_HEAD entry

@m-vo
Copy link
Member

m-vo commented Dec 2, 2017

Allthough I agree with @backbone87 's comment, why do we use array_unique here? Was this a fix for something else?

Regarding og/social images, it would be nice to have some abstraction for that. Contao modules itself add og tags in some places (e.g. image and description on a newsreader detail site) but the core does not support it by default in the BE (and that might be a good decisison). This is leaving opengraph extensions with the need to regex/tokenize the existing entries to eventually alter/overwrite them.

@discordier
Copy link
Contributor

The array_unique is there because of modules and content elements adding tags multiple times. Imagine 5 elements on the same page, each adding a certain head tag.
These tags (usually some script) should only be added once.
Therefore we can not change this without having a big bc break.

@fritzmg
Copy link
Contributor

fritzmg commented Dec 2, 2017

Regarding og/social images, it would be nice to have some abstraction for that. Contao modules itself add og tags in some places (e.g. image and description on a newsreader detail site) but the core does not support it by default in the BE (and that might be a good decisison). This is leaving opengraph extensions with the need to regex/tokenize the existing entries to eventually alter/overwrite them.

There are some plans: contao/core#8787 (comment)

@qzminski
Copy link
Member Author

qzminski commented Dec 4, 2017

@backbone87 that's a good idea, I will use it!

@discordier at some point that is true but if you have N elements on the same page, it's likely that each of the scripts should check and overwrite a certain head tag. That's not the case here because for the array_unique() function always the first occurrence of doubled item is preserved and the next are dropped. This might be a topic for a different discussion though.

@leofeyer
Copy link
Member

leofeyer commented Dec 7, 2017

I don't think we can do much about it; this is how array_unique() works. And as @discordier explained, we cannot stop using it. You have to stick with @backbone87's solution.

@leofeyer leofeyer closed this as completed Dec 7, 2017
leofeyer added a commit that referenced this issue Jan 21, 2020
… action (see #1225)

Description
-----------

Fixes #1214

Commits
-------

328511e0 Correctly determine the Ajax URL now that our forms no longer have an action
ba4f4933 Use the action attribute if there is one
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

6 participants