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

Duplicate ID "outbound-link-title" violates HTML5 #2627

Closed
1 task done
katelynsills opened this issue Sep 25, 2020 · 12 comments
Closed
1 task done

Duplicate ID "outbound-link-title" violates HTML5 #2627

katelynsills opened this issue Sep 25, 2020 · 12 comments
Labels
good first issue Good for newcomers Hacktoberfest Good issue for Hacktoberfest challenge

Comments

@katelynsills
Copy link

  • I confirm that this is an issue rather than a question.

Bug report

Steps to reproduce

  1. Create a site with several external links on one page.
  2. Build the site with Vuepress 1.6.0
  3. Attempt to validate the result as HTML5
  4. Validation will fail due to duplicate IDs whereas with Vuepress 1.5, it did not.

For a concrete example, please see this PR which attempts to upgrade to Vuepress 1.6.0 and uses a Github Action for HTML5 Validation.

What is expected?

Vuepress should be HTML5 compliant.

What is actually happening?

Duplicate IDs per page are being produced

Other relevant information

8d10119 Seems to be the commit that introduced this problem.

  • Output of npx vuepress info in my VuePress project:
Environment Info:

  System:
    OS: macOS 10.15.6
    CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
  Binaries:
    Node: 14.11.0 - ~/.nvm/versions/node/v14.11.0/bin/node
    Yarn: 1.22.5 - /usr/local/bin/yarn
    npm: 6.14.8 - ~/.nvm/versions/node/v14.11.0/bin/npm
  Browsers:
    Chrome: 85.0.4183.121
    Edge: Not Found
    Firefox: Not Found
    Safari: 14.0
  npmPackages:
    @vuepress/core:  1.6.0 
    @vuepress/theme-default:  1.6.0 
    vuepress: ^1.6.0 => 1.6.0 
  npmGlobalPackages:
    vuepress: Not Found
@pbek
Copy link

pbek commented Sep 27, 2020

Thank you for bringing that up, @katelynsills. I just found out about VuePress today and the first thing I did after a yarn run build on the boilerplate webpage was uploading the html files to https://validator.w3.org/nu/#file to check the HTML5 validity. 😁

image

Google will not like that at all. 😀

@d-pollard d-pollard added good first issue Good for newcomers Hacktoberfest Good issue for Hacktoberfest challenge labels Oct 2, 2020
@adico1
Copy link
Contributor

adico1 commented Oct 8, 2020

I can take that.
do you think it will be a good solution to just remove the id. I didn't find its being used.

@adico1
Copy link
Contributor

adico1 commented Oct 8, 2020

  1. removing the id remove the first error.

but brings up another one
Screen Shot 2020-10-08 at 20 59 46

  1. taking the same action and removing the attribute aria-labelledby="outbound-link-title" fixes the second bug
    but I'm not sure if that case is fits as solution.

Let me know if that is ok, otherwise I can use the following link which suggest a solution for such problem
sdras/vue-sample-svg-icons#2

adico1 added a commit to adico1/vuepress that referenced this issue Oct 8, 2020
@adico1
Copy link
Contributor

adico1 commented Oct 8, 2020

i've added a PR removing both:

  1. the attribute 'id' from title which is unnecessary
  2. the aria-labelledby from SVG which i guess can be removed

#2648

I will add another PR later with random id solution

adico1 added a commit to adico1/vuepress that referenced this issue Oct 8, 2020
@adico1
Copy link
Contributor

adico1 commented Oct 8, 2020

the current PR doesn't remove any attribute but rather adds a random id to the id attributes
#2649

@ktquez
Copy link

ktquez commented Oct 8, 2020

The ideal would be to add aria-hidden="true" to the svg icon and add visually hidden text with some class of type sr-only.

adico1 added a commit to adico1/vuepress that referenced this issue Oct 8, 2020
@adico1
Copy link
Contributor

adico1 commented Oct 9, 2020

add a new PR that fix the issue by @ktquez suggestion
#2650

@ktquez
Copy link

ktquez commented Oct 9, 2020

@adico1
Awesome! Thanks for this fix

@adico1
Copy link
Contributor

adico1 commented Oct 9, 2020

ty for the comment, actually I think its the best solution because its also scalable.

adico1 added a commit to adico1/vuepress that referenced this issue Oct 9, 2020
@adico1
Copy link
Contributor

adico1 commented Oct 13, 2020

Hi @d-pollard,
When can I get additional review?

@d-pollard
Copy link
Collaborator

Right now! Thanks for this, going to review it.

@d-pollard
Copy link
Collaborator

Approved, pending review from Ben and this will probably be merged in later today! Excellent work everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers Hacktoberfest Good issue for Hacktoberfest challenge
Projects
None yet
Development

No branches or pull requests

5 participants