Conversation
…into release/first_version
rimi-itk
left a comment
There was a problem hiding this comment.
I think this works, but some parts are hard to understand. Some comments added and questions asked.
| $requiredCookies = $supportedProviders[$providerKey]['requiredCookies']; | ||
|
|
||
| if (!empty($requiredCookies) && isset($videoArray['iframe'])) { | ||
| $videoArray['iframe'] = str_replace(' src="', ' src="" data-category-consent="' . $requiredCookies . '" data-consent-src="', $videoArray['iframe']); |
There was a problem hiding this comment.
This probably works, but it makes a lot of assumptions on the structure of the iframe HTML.
There was a problem hiding this comment.
It does assume there is a ' src="' attribute. What would be a better approach?
There was a problem hiding this comment.
Using string replacements to manipulate HTML/XML is never the best solution. If the oEmbed provider suddenly returns something that not a iframe element or starts returning something that contains more than one src attribute, the replacement will not work as expected.
The best way to do the manipulations would be to use https://www.php.net/manual/en/class.dom-htmldocument.php to load the HTML and then use DOM operations to add/change/remove attributes. This would also make it easy to check that we actually have an iframe element and report an error (like “Unhandled oEmbed code”, say) if not.
Using HTMLDocument will require a little more work (programming), but makes the solution much more clear, e.g. something along the lines of
function consentifyOEmmed(string $content, array $requiredCookies) {
$dom = \Dom\HTMLDocument::createFromString('<!DOCTYPE html><html><body>'.$content.'</body></html');
$element = $dom->body->firstElementChild;
if ('IFRAME' === $element->nodeName && $element->hasAttribute('src')) {
$src = $element->getAttribute('src');
$element->setAttribute('src', '');
$element->setAttribute('data-consent-src', $src);
$element->setAttribute('data-category-consent', implode(' ', $requiredCookies));
} else {
die(sprintf("Unhandled oEmbed code: %s\n", $content));
}
return $dom->saveHtml($element);
}Co-authored-by: Mikkel Ricky <rimi@aarhus.dk>
Co-authored-by: Mikkel Ricky <rimi@aarhus.dk>
Co-authored-by: Mikkel Ricky <rimi@aarhus.dk>
Co-authored-by: Mikkel Ricky <rimi@aarhus.dk>
Co-authored-by: Mikkel Ricky <rimi@aarhus.dk>
Co-authored-by: Mikkel Ricky <rimi@aarhus.dk>
| $requiredCookies = $supportedProviders[$providerKey]['requiredCookies']; | ||
|
|
||
| if (!empty($requiredCookies) && isset($videoArray['iframe'])) { | ||
| $videoArray['iframe'] = str_replace(' src="', ' src="" data-category-consent="' . $requiredCookies . '" data-consent-src="', $videoArray['iframe']); |
There was a problem hiding this comment.
Using string replacements to manipulate HTML/XML is never the best solution. If the oEmbed provider suddenly returns something that not a iframe element or starts returning something that contains more than one src attribute, the replacement will not work as expected.
The best way to do the manipulations would be to use https://www.php.net/manual/en/class.dom-htmldocument.php to load the HTML and then use DOM operations to add/change/remove attributes. This would also make it easy to check that we actually have an iframe element and report an error (like “Unhandled oEmbed code”, say) if not.
Using HTMLDocument will require a little more work (programming), but makes the solution much more clear, e.g. something along the lines of
function consentifyOEmmed(string $content, array $requiredCookies) {
$dom = \Dom\HTMLDocument::createFromString('<!DOCTYPE html><html><body>'.$content.'</body></html');
$element = $dom->body->firstElementChild;
if ('IFRAME' === $element->nodeName && $element->hasAttribute('src')) {
$src = $element->getAttribute('src');
$element->setAttribute('src', '');
$element->setAttribute('data-consent-src', $src);
$element->setAttribute('data-category-consent', implode(' ', $requiredCookies));
} else {
die(sprintf("Unhandled oEmbed code: %s\n", $content));
}
return $dom->saveHtml($element);
}
Includes