search

epiphyt / embed-privacy

Embed Privacy prevents loading of embedded external content and allows your site visitors to opt-in.
https://epiph.yt/en/embed-privacy/
GNU General Public License v2.0
19 stars 12 forks source link

Fix invalid HTML through style tags #198

Closed MatzeKitt closed 9 months ago

MatzeKitt commented 1 year ago

Bug/Problem

https://wordpress.org/support/topic/inline-styling-appears-to-break-html-validation/

Steps to reproduce

Validate any page Embed Privacy is active on

Version

1.8.0

Link

No response

Environment info

No response

Code of Conduct

MatzeKitt commented 9 months ago

Unfortunately, this is probably not fixable since the replacement takes place after the wp_head action and thus any style is added to the footer. I would need to process the content before its output, which is problematic since the necessary filter haven’t been executed at this point yet.

arnowelzel commented 9 months ago

As far I see it, the issue can be fixed. There is no need to output styles which refer to a DIV element inside that very element instead of using inline styles.

So instead of doing this:

<div data-embed-id="foobar">
...
<style>
[data-embed-id="foobar"] { mystyle; }
</style>
</div>

Do this:

<div data-embed-id="foobar" style="mystyle">
...
</div>
MatzeKitt commented 9 months ago

Inline styles are highly discouraged since they’re hard to overwrite and rather unperformant at all.

arnowelzel commented 9 months ago

I did not realize, that there is already a commit for this which tried to solve that (https://github.com/epiphyt/embed-privacy/commit/52efe794b8c8beb2b2cb6f53d9a12d79086253df).

arnowelzel commented 9 months ago

I modified the code for my own website, example: https://arnowelzel.de/en/pinecil-v2-with-bluetooth

About performance:

Every style is only valid for one single element only. There is no performance gain in putting all the dynamically created rules into a separate <style> element in the document instead of having the same rules as inline style in the one single element which is affected by the style.

The browser has to parse every single style on every page anyway since the styles are not global rules which are loaded from an external and thus cacheable resource like the themes style.css. They use a unique selector like [data-embed-id="oembed_<ID>"] where <ID> is a unique ID and not a reusable selector like .embed-privacy-inner or .embed-privacy-logo.

So - yes in general I agree having styles which would get repeated multiple times as inline style makes no sense and should be replaced with a class. But in this case I think, it makes no difference at all.