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

Usage of duplicate id values in nav menu #4256

Closed
jankohoener opened this issue Jun 24, 2024 · 5 comments · Fixed by #4259
Closed

Usage of duplicate id values in nav menu #4256

jankohoener opened this issue Jun 24, 2024 · 5 comments · Fixed by #4259
Assignees
Labels
bug This label could be used to identify issues that are caused by a defect in the product. Priority-Medium Expected resolution time - up to 1 month. released Indicate that an issue has been resolved and released in a particular version of the product.

Comments

@jankohoener
Copy link

Description

Using Neve without any plugins enabled on our staging site adds to the HTML tree of the nav menu duplicate id values, once for the desktop view and once for the mobile view. An example is id="secondary-menu" if used both on desktop and mobile view.
This leads to errors by the W3C Validator and potential downgrade of SEO. It's also semantically incorrect to use the same id for two elements. And it may lead to problems with CSS, where the same selector is applied both on desktop and mobile view.
The id's should either be changed into a class or they should be renamed.

Step-by-step reproduction instructions

  1. Create Wordpress site with Neve.
  2. Add a header element to both desktop and mobile view (for example a secondary menu)
  3. Save
  4. The HTML source will contain duplicate id values (e.g. secondary-menu)

(example of a staging site with Neve only, no plugins, is at https://www.maxsel.de/technicalseo/)

Screenshots, screen recording, code snippet or Help Scout ticket

No response

Environment info

https://pastebin.com/Hw1e9caF

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 Jun 24, 2024
@selul selul added the Priority-Medium Expected resolution time - up to 1 month. label Jun 26, 2024
@selul
Copy link
Contributor

selul commented Jun 26, 2024

hi @jankohoener,

Thanks for reporting, we will look into it.

@pirate-bot
Copy link
Collaborator

🎉 This issue has been resolved in version 3.8.10 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@pirate-bot pirate-bot added the released Indicate that an issue has been resolved and released in a particular version of the product. label Jul 31, 2024
@jankohoener
Copy link
Author

The duplicate id secondary_menu is indeed gone from our site. However, other navigation elements that are used on both desktop and mobile still have duplicate id's for example text, nav menu, custom HTML.

@vytisbulkevicius
Copy link
Contributor

Hi @jankohoener,

I'm trying to replicate the issue, just to confirm - are you getting duplicate values on header or footer?
I checked your website and I don't see duplicate ids in the header, well I see for the secondary menu but maybe you haven't updated the theme yet? I don't see it for other elements.

I see it in the footer, though, can you share with me a screenshot from Customizer of how your footer is built? I will also create a new issue since this one was already released and for secondary menu it was fixed. Thanks!

@vytisbulkevicius
Copy link
Contributor

@jankohoener,

I'm closing this issue but feel free to reply or create a new one if you can still replicate the issue you mentioned. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This label could be used to identify issues that are caused by a defect in the product. Priority-Medium Expected resolution time - up to 1 month. released Indicate that an issue has been resolved and released in a particular version of the product.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants