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.

Sign up for GitHub

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

Jump to bottom

CSS <style> added to <body> by nav_walker.php #4279

Open
jankohoener opened this issue Jul 30, 2024 · 3 comments
Open

CSS <style> added to <body> by nav_walker.php #4279

jankohoener opened this issue Jul 30, 2024 · 3 comments
Labels
bug This label could be used to identify issues that are caused by a defect in the product.

Comments

@jankohoener
Copy link

jankohoener commented Jul 30, 2024 •
edited
Loading

Description

The file nav_walker.php adds two inline <style>'s after the header by the hook neve_after_header_wrapper_hook, that means as a child element of the < body > under certain conditions. This gives errors by the W3C Validator, as <style> elements are only allowed as child of the < head > and thus reduces technical SEO. Both inline styles are added on our website to the < body >.

Alternatively, if the < style > is only meant for the subtree, you could add the boolean scoped attribute.

Step-by-step reproduction instructions

Default installation of Neve contains on our staging site already one <style> element added to the .

Screenshots, screen recording, code snippet or Help Scout ticket

No response

Environment info

No response

Is the issue you are reporting a regression

No
@jankohoener jankohoener added the bug This label could be used to identify issues that are caused by a defect in the product. label Jul 30, 2024
@vytisbulkevicius
Copy link
Contributor

@girishpanchal30,

I'm assigning this to you to check. Ideally we should move inline CSS to tag but we should be careful and test it if it doesn't break anything this way and if we have the information needed at the time at wp_head to inject it there.

Thanks!

@vytisbulkevicius vytisbulkevicius added the Priority-Medium Expected resolution time - up to 1 month. label Aug 6, 2024
@girishpanchal30
Copy link
Contributor

@vytisbulkevicius I've reviewed this issue and I need your suggestion.

The wp_nav_menu function is called after the wp_head action so I believe we can't move the style through the Nav_Walker class.

We can move style in the wp_head action globally without checking any logical conditions.

For example, here style is added for child menu items.

Could you please let me know if I should move the style into wp_head globally?

Thanks

@selul
Copy link
Contributor

selul commented Aug 7, 2024

Since the scoped attribute is no longer supported, and loading CSS in the head always impacts the page weight, even if the CSS is not used, I propose that we dont do any action for now. We should wait until we can figure out a better and more scalable solution.

@vytisbulkevicius vytisbulkevicius removed the Priority-Medium Expected resolution time - up to 1 month. label Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug This label could be used to identify issues that are caused by a defect in the product.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@selul @jankohoener @vytisbulkevicius @girishpanchal30