Lighthouse SEO
Crawling and IndexingTo appear in search results, crawlers need access to your app.
Links are not crawlable
Search engines may use `href` attributes on links to crawl websites. Ensure that the `href` attribute of anchor elements links to an appropriate destination, so more pages of the site can be discovered. Learn More
Uncrawlable Link
a#main-content
| Comment | File | Size | Author |
|---|---|---|---|
| #50 | interdiff-47_50.txt | 4.75 KB | gauravvvv |
| #50 | 3222236-50.patch | 4.61 KB | gauravvvv |
| #47 | 3222236-claro-theme.png | 239.95 KB | pradipmodh13 |
| #47 | 3222236-olivero-theme.png | 309.49 KB | pradipmodh13 |
| #47 | 3222236-47.patch | 4.28 KB | pradipmodh13 |
Issue fork drupal-3222236
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
larowlanAre you using claro as your front end theme?
Comment #3
hop commentedYes, why not?
Is it forbidden or is it not allowed?
Comment #4
larowlanNo, however its not common - as its an Admin theme
Comment #5
jeroentSimilar issue in webform: #3214340: Links are not crawable error in Lighthouse
Comment #6
jeroentAlso, this is not only limited to Claro but is a problem in
page.html.twigin the following themes:And the page.html.twig template provided by the system module.
So this:
<a id="main-content" tabindex="-1"></a>should be updated to:
<span id="main-content" tabindex="-1"></span>On the webform issue, tabindex was also removed. But I'm not sure if that is necessary.
Comment #7
jeroentComment #9
saranyamariappan commentedI am working on it.
Comment #10
saranyamariappan commentedBelow patch is fixes provided only for Claro Theme.
Comment #11
jeroentBack to needs work for the other themes.
Comment #12
longwaveThe point of this is so we have an ID attribute for the browser to scroll or tab to, right? So if we're dropping the
<a>here, can we just put this directly on the<main>element?I'm unsure of the accessibility implications of any of this, but if we can replace an anchor with a span, I'm not sure why we need the span at all.
Comment #13
saranyamariappan commentedI am working on it, for other core themes.
Comment #14
longwavehttps://webaim.org/techniques/skipnav/ suggests that
<main id="main-content">is perfectly OK.Comment #15
saranyamariappan commentedUpdated the "a" tag to "span" in all page.html.twig files on all the core themes. This patch includes the changes made on #10 patch as well. This going to be the consolidated one.
Comment #17
meenakshi_j commentedFixed the fails of #15
Comment #18
longwaveTentatively marking RTBC as the patch solves the problem at hand, but would like an accessibility review so we can see if #12 if viable as I feel that is cleaner than using spans.
Comment #19
alexpottA links without an href are valid html - see http://w3c.github.io/html-reference/a.html - is this really a bug?
Comment #20
JeremyFrench commentedThe HTML is valid, but it has a negative SEO impact, according to lighthouse (made by google so they should know). This means potentially every site which uses one of these themes will have an inbuilt negative SEO impact on every page.
Comment #21
larowlanAccording to https://css-tricks.com/a-deep-dive-on-skipping-to-content/ we should keep it as an anchor tag, but add a href that links back to itself
Comment #22
vsujeetkumar commentedAddressed #21, Patch created.
@larowlan I have added href="#main-content" in anchor tag, I have bit confuse about it is the correct way to call back to itself?, Please have a look and advise.
Comment #23
longwaveAccording to https://css-tricks.com/a-deep-dive-on-skipping-to-content/ #22 is the right approach, however that link also suggests including text inside the skip link target, we currently have no text. Unsure if that is in scope for this issue or not.
After applying #22 there are still a few places that use <a id> with no href, these should all be fixed together:
Comment #24
vsujeetkumar commentedAs mentioned in #23, Added changes in some more places.
Comment #25
larowlanI think we should add text in the anchor whilst we're here.
Comment #26
alexpott@larowlan won't we then need to add the visibility-hidden class? I think maybe discussing the BC impacts of doing that is worth it and so making that part of the change in follow-up seems a good idea.
Comment #27
larowlanYeah, I hadn't considered that - and its even more involved than adding visibility-hidden
Here's a screenshot from the example in that article of when the anchor has focus
So that will be something that takes time.
I also found this resource https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#skip_links which has it linking to the main element instead of an a
Comment #28
longwave@larowlan I raised the possibility of putting the ID on
<main>instead of<a>in #12/14 but am unsure of the accessibility implications of doing so, so tagged for that in #18. There is also a minor issue that<main>already has a different ID in some templates, although there are child elements we could put the attribute on instead.Comment #30
noonoos commented<a href="#main-content" id="main-content" tabindex="-1"></a>as per #24Works for Bootstrap Barrio. SEO 93=>100
Comment #33
dpagini commentedJust noticing this patch is only for 9.3.x, so it should be updated. If this DOES get re-rolled however, is this direction the right approach to get this accepted to core? This makes sense to me at least as far as addressing the OP.
Comment #37
dpagini commentedRe-rolled for 10.1.x. I am using the previous patch, and now this new re-roll, and it's fixing my Google PageSpeed complaint, so I'm going to move this right to RTBC. Hopefully that will get someone from the core team to look at this again and give some direction on what's needed here to solve this problem...?
Comment #38
dpagini commentedComment #39
dpagini commentedI should add that I am getting this Lighthouse "ding" now:
Which I think is what is mentioned in #25.
Comment #40
xjmThere doesn't seem to be a specific reason that the earlier approach with
<span>was abandoned, other than that the markup of<a>withouthrefis valid and used to be very common.I think we need that accessibility review to go forward here. I wouldn't want to degrade the navigational experience for SEO if there's a better way to improve the markup.
Comment #42
dpagini commented@xjm - who would do that review? Can we request that review from those individuals?
Putting up an alternate merge request which I believe uses the
<span>method you referred to.Comment #43
smustgrave commentedPosted to the #accessibility channel to see if they can take a look
But if we are changing all the theme templates won't we need a change record if any thing base themed off one of these?
Comment #44
dgwolf commentedHi, Lighthouse complains that links are not crawlable in our new site on D9.5.7 with the default Olivero theme that doesn't show up in Google search so far. It specifies
"Uncrawlable Link", and
I am a bit lost as to which patches have been applied in 9.5.7, if any, and what workaround could currently actually be applied with the least damage resulting elsewhere. Or is an upgrade to D10 the best solution?
Comment #45
pradipmodh13 commentedHello Folks,
In Drupal 10.1.x-dev anchor tag comes with href attribute. So no need to work.
For ref attached screenshot. So we can move this bug to needs review.
Comment #46
pradipmodh13 commentedHello Folks,
In Drupal 10.1.x-dev anchor tag comes with href attribute. So no need to work.
For ref attached screenshot. So we can move this bug to needs review.
Comment #47
pradipmodh13 commentedAfter looking further, it was discovered that the anchor tag's href property was missing, thus href was added in accordance with usual practise.
After applying the fix, I verified that the lighthouse reports were error-free.
Please review
Comment #48
longwave#47 is wrong, now the anchor is a link to itself.
I still think the simplest solution is to put the ID directly on the
<main>element.WebAIM suggests that this would be OK: https://webaim.org/techniques/skipnav/
Comment #50
gauravvvv commentedAddressed feedback from #48, Attached interdiff with 47, please review
Comment #51
dpagini commentedI came across this other core issue regarding these same links, and I think a solution should probably consider both of these issues...?
Comment #52
smustgrave commentedSeems latest change caused failures.
Comment #53
dpagini commentedWhat do you mean by that, @smustgrave?
Comment #54
smustgrave commented#50 didn't fully pass. Caused some nightwatch errors I believe.