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

Include links with no text as "non-descriptive" in SEO audit #6495

Closed
patrickhulce opened this issue Nov 6, 2018 · 15 comments
Closed

Include links with no text as "non-descriptive" in SEO audit #6495

patrickhulce opened this issue Nov 6, 2018 · 15 comments

Comments

@patrickhulce
Copy link
Collaborator

patrickhulce commented Nov 6, 2018

Summary
Split out from #6483 (comment) per @rviscomi 's request. We should consider including all links that don't have any text content in our non-descriptive check. A link that doesn't have any text at all is just as non-descriptive as these other links :)

I think the main concern is that this will start including a bunch of things we don't foresee.

@rviscomi
Copy link
Member

rviscomi commented Nov 7, 2018

Thanks for filing this @patrickhulce! Will investigate.

@brendankenny brendankenny changed the title Include links with no text as "non-descriptive" in SEO audit expand list of "non-descriptive" link text in SEO link-text audit Jan 30, 2019
@brendankenny brendankenny changed the title expand list of "non-descriptive" link text in SEO link-text audit Include links with no text as "non-descriptive" in SEO audit Jan 30, 2019
@rviscomi rviscomi assigned rviscomi and mattzeunert and unassigned rviscomi Feb 21, 2019
@rviscomi
Copy link
Member

Assigning to Matt to start implementing and testing on popular sites. Let's see what the new failure rate is before releasing it.

@mattzeunert
Copy link
Contributor

mattzeunert commented Feb 22, 2019

Ran on 100 sites from here.

Before After
Failure rate 38% 81%
Average item count 0.75 10.5
Median item count 0 3

Right now the AnchorElements artifact uses innerText as the link text, which means the text is empty if the link is hidden. I've used textContent for now. If we don't want to count invisible elements we can detect that in the gatherer.

@rviscomi
Copy link
Member

Hmm I think we should probably ignore hidden text to be consistent with the users' experience.

I'm concerned about the big spike in the failure rate though. Can you remind me when a warning gets triggered as opposed to a failure? Looked at the audit source code but didn't see a distinction. It might be worthwhile to warn about empty anchors rather than failing the audit. Not sure.

@mattzeunert
Copy link
Contributor

mattzeunert commented Feb 22, 2019

Sorry, I meant items rather than warnings. Having one failing item in the audit result will cause the audit to fail.

Most of the time the images contain an image instead of text, sometimes with an aria-label.

@rviscomi
Copy link
Member

Maybe we should be ignoring a > img cases? If they were to cause the audit to fail, what would the recommended fix even be: replace the image with text? put text next to the image? Not sure if img[alt] attributes are used instead, but I do know that aria-label attributes are not. I'll consult the WTAs.

@rviscomi
Copy link
Member

Martin (@AVGP) suggested that if the anchor contains an image, we fall back to its alt text. Could we see how the failure rate changes with that implementation?

@mattzeunert
Copy link
Contributor

This is if we include count image alt attributes as text:

Before After
Failure rate 37% 70%
Average item count 0.74 7.0
Median item count 0 1.5

Actually both of those numbers are inflated because I removed the visibility check. This is the result if we ignore invisible elements again:

Before After
Failure rate 27% 63%
Average item count 0.57 3.0
Median item count 0 1

@paulirish
Copy link
Member

I'm pretty scared to make this policy change. We'd have to look at all the possible failures and determine if adding text is a reasonable solution for them all.

Would this be an improvement for accessibility or SEO? And why?

It seems like no one is strongly advocating for it right now, so let's shelve it?

@AVGP
Copy link
Collaborator

AVGP commented Feb 25, 2019 via email

@rviscomi
Copy link
Member

SGTM. This is a low priority change so we have time to vet it properly.

@mattzeunert
Copy link
Contributor

Here's the output from running the script.

Quite a few of them have an aria-label or a title, but I guess those don't count for SEO?

@connorjclark
Copy link
Collaborator

@AVGP @umaar is this still a priority in your opinions?

@AVGP
Copy link
Collaborator

AVGP commented Jun 9, 2020

We figured it isn't that pressing of an issue. If in doubt, I'd skip this.

@brendankenny
Copy link
Member

Great, thanks! We can always revisit if it turns out we do have a blindspot here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants