Entity link annotations in HTML head are not valid HTML

I'm working on this issue while sprinting in Barcelona.

Note that the rel attributes has defined keywords which we can use, see: http://www.w3.org/TR/html5/links.html#linkTypes. There is the possibility to propose new keywords, see http://microformats.org/wiki/existing-rel-values. But I don't think this is applicable here.

Will investigate this further.

The non-validating code is found in the HTML head of the node page when the user is allowed to edit it:

<link rel="delete-form" href="/node/1/delete" />
<link rel="edit-form" href="/node/1/edit" />
<link rel="version-history" href="/node/1/revisions" />
<link rel="revision" href="/node/1" />

The code was introduced in https://www.drupal.org/node/2281645.

Updated summary with latest findings.

Marked as duplicate of #2406533: edit-form, delete-form etc. <link> tags added on /node/{node} are invalid according to W3C Validator

The related issue only deals with some links, not all of them. Right now, the "revision" link is still there and still invalid.

To elaborate a bit, this is happening in \Drupal\node\Controller\NodeViewController::view().
The core, list all uriRelationships of the node (ie. everything in the "links" section of its annotation) then checks if the user has access to the given URL. The "revision" URL being translated the node URL when viewing the current revision, the user has access to it so this link tag is put in the head and it breaks the W3C validator.

Hello, it would be great to have a way to turn these off. I'm working on a site migration project, and I'm using link checker software to search for broken links. It finds these in the head and reports them as false negatives, because they return 403 forbidden (as the link checker is "browsing" as anonymous.)

I'm keen to see any progress on this or possible solutions, as W3 validation is a request/requirement of many govt/enterprise sites, so seems like good idea to make core validate as much as possible.

I agree with @edmonkey here. Really looking forward to W3C valid html code.
Hoping that the issue is resolved soon.

Until this issue gets resolved, anyone coming here could try the module Unset HTML Head Link.

I have filtered all valid tags in the uriRelationships method

Patch for drupal 8.8

Updated patchfiles to fix ViewUI Error

Updated test cases. Removed irrelevant test that is checking if node edit link will be shown.

Re-rolled this patch for 8.7.

Re-rolled 8.7 patch

Re-rolled 8.7 , this time with the correct naming conventions!

Just had a look at and the full list of rel values is:

$validRelList = [
    'alternate',
    'archives',
    'author',
    'bookmark',
    'canonical',
    'dns-prefetch',
    'external',
    'first',
    'help',
    'icon',
    'import',
    'index',
    'last',
    'license',
    'manifest',
    'modulepreload',
    'next',
    'nofollow',
    'noopener',
    'noreferrer',
    'opener',
    'pingback',
    'preconnect',
    'prefetch',
    'preload',
    'prerender',
    'prev',
    'search',
    'shortcut icon',
    'shortlink',
    'stylesheet',
    'tag',
  ];

A couple of those are obsolete...

Rerolled https://www.drupal.org/project/drupal/issues/2454915#comment-13151540 against 8.8.0

Removed the revision link from the test, as that one will get filtered out by this change.

1. +++ b/core/modules/node/src/Controller/NodeViewController.php
   @@ -128,4 +128,42 @@ public function title(EntityInterface $node) {
   +   *   The unchecked rel attribute
   

   Add "."

2. +++ b/core/modules/node/src/Controller/NodeViewController.php
   @@ -128,4 +128,42 @@ public function title(EntityInterface $node) {
   +    // List of all supported rel tags
   

   Add "."

3. +++ b/core/modules/node/tests/src/Functional/NodeViewTest.php
   @@ -30,10 +30,10 @@ public function testHtmlHeadLinks() {
   +    $this->assertEmpty($result,'Version history is no valid link relation and should not be visible for guests');
   

   Add " " after comma.

4. +++ b/core/modules/node/tests/src/Functional/NodeViewTest.php
   @@ -30,10 +30,10 @@ public function testHtmlHeadLinks() {
   +    $this->assertEmpty($result,'Edit form is no valid link relation and should not be visible for guests');
   

   Add " " after comma.

5. +++ b/core/modules/node/tests/src/Functional/NodeViewTest.php
   @@ -43,31 +43,10 @@ public function testHtmlHeadLinks() {
   +    $this->assertEmpty($result,'Version history is no valid link relation and should not be visible for users');
   

   Add " " after comma.

6. +++ b/core/modules/node/tests/src/Functional/NodeViewTest.php
   @@ -43,31 +43,10 @@ public function testHtmlHeadLinks() {
   +    $this->assertEmpty($result,'Edit form is no valid link relation and should not be visible for guests');
   

   Add " " after comma.

Fixed codestyle remarks.

Thanks to @ducktape,

Patch #31 also applies cleanly to Drupal 8.8.x
Patch #31 also applies cleanly to Drupal 9.0.x
Patch #31 also applies cleanly to Drupal 9.1.x

+1 for this! Thanks, we need this to pass w3c validation testing on https://validator.w3.org/nu/

for now we are adding this patch to our build

our build has a very long list of core patches now to which I will be pinging for status updates at some point.

+1

1. +++ b/core/modules/node/src/Controller/NodeViewController.php
   @@ -128,4 +128,42 @@ public function title(EntityInterface $node) {
   +  public function isValidRel($rel) {
   

   Is there a reason we made this 'public' - I'm not sure we want to make this a public API.

2. +++ b/core/modules/node/src/Controller/NodeViewController.php
   @@ -128,4 +128,42 @@ public function title(EntityInterface $node) {
   +    return in_array($rel, $validRelList);
   

   we're comparing string here, so let's use the third argument here

3. +++ b/core/modules/node/tests/src/Functional/NodeViewTest.php
   @@ -30,10 +30,10 @@ public function testHtmlHeadLinks() {
   -    $this->assertEmpty($result, 'Version history not present for anonymous users without access.');
   +    $this->assertEmpty($result, 'Version history is no valid link relation and should not be visible for guests');
   ...
   -    $this->assertEmpty($result, 'Edit form not present for anonymous users without access.');
   +    $this->assertEmpty($result, 'Edit form is no valid link relation and should not be visible for guests');
   

   these changes here are out of scope

4. +++ b/core/modules/node/tests/src/Functional/NodeViewTest.php
   @@ -43,31 +43,10 @@ public function testHtmlHeadLinks() {
   +    $this->assertEmpty($result, 'Version history is no valid link relation and should not be visible for users');
   ...
   +    $this->assertEmpty($result, 'Edit form is no valid link relation and should not be visible for guests');
   

   these aren't valid English, but I don't think they add anything, so let's just leave these as assertEmpty($result) and let phpunit generate its default message

Voilà c'est fait.

interdiff and new patch as per request.

Thanks for the quick turnaround joseph ... #36 works as well for us as #31 did. RTBC+1

+1 This patch works as expected, very good. The HTML output is now much cleaner and compliant.

@larowlan, your suggested changes were added to the patch, is there anything left for us to do here? Or can this be committed right away?

#36 fixes w3c validation error for me.
Thank you @joseph!

Drupal 8.9.7

+1 any help needed anymore for inclusion ?
Adding #2467827: [META] W3C validation for Drupal Core as parent issue for reference.

Closed a duplicate so added credit for isholgueras.

This looks ready to me, there's one suggestion here about type-hints, and finally a fix for type-safety with in_array

Fine to self RTBC those changes because they're minor

1. +++ b/core/modules/node/src/Controller/NodeViewController.php
   @@ -123,4 +123,42 @@ public function title(EntityInterface $node) {
   +  private function isValidRel($rel) {
   

   We can use a scale type-hint and a return-type here now (new code should use return/scalar types), ie private function isValidRel(string $rel) : bool

2. +++ b/core/modules/node/src/Controller/NodeViewController.php
   @@ -123,4 +123,42 @@ public function title(EntityInterface $node) {
   +    return in_array($rel, $validRelList, FALSE);
   

   the third argument here should be TRUE, these are strings, and the rel is a string, so we should be using the type-safe version

Addressed #47.1 and .2.

Fine to self RTBC those changes because they're minor

RTBC-ing myself feels weird, but also good... ;)

I used your patch and did not notice anything that was not expected.
However, reading the code, here are a few nitpicks I see. They are all about missing authorized rel attributes according to the documentation.

1. +++ b/core/modules/node/src/Controller/NodeViewController.php
   @@ -123,4 +123,42 @@ public function title(EntityInterface $node) {
   +   * Checks if rel attribute is w3c.org valid
   

   I suggest to include something like:
   @see https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/rel

   to offer a link to official documentation with listed valid attributes. Also following this documentation, some rels are missing in the list below.

2. +++ b/core/modules/node/src/Controller/NodeViewController.php
   @@ -123,4 +123,42 @@ public function title(EntityInterface $node) {
   +      'canonical',
   

   dns-fetch exists here too. Should we include it ?

3. +++ b/core/modules/node/src/Controller/NodeViewController.php
   @@ -123,4 +123,42 @@ public function title(EntityInterface $node) {
   +      'shortcut icon',
   

   why not keep this alphabetically ?

4. +++ b/core/modules/node/src/Controller/NodeViewController.php
   @@ -123,4 +123,42 @@ public function title(EntityInterface $node) {
   +      'noopener',
   

   "opener" also exists

5. +++ b/core/modules/node/src/Controller/NodeViewController.php
   @@ -123,4 +123,42 @@ public function title(EntityInterface $node) {
   +      'prefetch',
   

   "preconnect" also exists, as well as "prerender"

6. +++ b/core/modules/node/src/Controller/NodeViewController.php
   @@ -123,4 +123,42 @@ public function title(EntityInterface $node) {
   +      'stylesheet',
   

   "search" is missing before

All points raised by @Dom in #50 seem valid to me.

All addressed in attached patch.

At least on my humble point of view: RTBC

Yes we are also using this for w3c compliance, thanks to everyone above.

+++ b/core/modules/node/src/Controller/NodeViewController.php
@@ -123,4 +123,49 @@ public function title(EntityInterface $node) {
+  private function isValidRel($rel) : bool {

Note to self: this won't cause disruption for implementations that extend NodeViewController because it is private

See https://3v4l.org/Q9UOD vs https://3v4l.org/Utcql

Saving issue credits

Committed 9d4138c and pushed to 9.2.x. Thanks!

Moving to 9.1.x for possible backport, will get a second opinion

Discussed this with @alexpott who pointed out there were two other issues where we'd decided not to fix this.

Linking those here.

Reverted in the meantime whilst we discuss those.

#2406533: edit-form, delete-form etc. <link> tags added on /node/{node} are invalid according to W3C Validator

Quoting @Dom. from #2468047-3: Header links makes extensive use of non W3C compliant rel attributes

To declare which meta terms are used in the document, instead register the names as meta extensions. To trigger specific UA behaviors, use a link element instead.

The list at https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/rel is not complete.

There are rel attributes that are part of other standards. To name a handful:

• https://www.w3.org/TR/webmention/#sender-discovers-receiver-webmention-e... → rel="webmention"
• https://indieauth.com/ → rel="me"
• https://micropub.spec.indieweb.org/ → rel="shortlink", rel="syndication" and rel="micropub"
• https://indieweb.org/Microsub-spec → rel="authorization_endpoint", rel="token_endpoint" and rel="microsub"
• https://tools.ietf.org/html/rfc5988 -> many examples, including rel="previous" and rel="payment"
• https://w3c.github.io/websub/ → rel="hub" and rel="self"

Long story short, I believe isValidRel() needs to be removed. I'm bumping this to major as this most likely breaks dozens of contributed modules.

Looks like the patch was reverted so changing the priority back to 'Normal'.

EDIT: Snap! Crossing posts...
Nothing to see here, move along!

Most probably this is my last post on d.o. since I'm going to disagree with the BDFL @Dries here...
I'm expecting the unmarked vans to pull up across my house any minute now. 😉

Long story short, I believe isValidRel() needs to be removed. I'm bumping this to major as this most likely breaks dozens of contributed modules.

As far as I can see isValidRel() is introduced in this patch, so there's no Contrib depending on it, since Contrib doesn't know about it.

So I think we can scale this down to priority "Normal" again?

(Not doing so, since I definitely don't want the people in the unmarked vans to pull out their stun guns...)

Just dropped by to say I loved comment #61 :)

+++ b/core/modules/node/src/Controller/NodeViewController.php
@@ -123,4 +123,49 @@ public function title(EntityInterface $node) {
+  /**
+   * Checks if rel attribute is w3c.org valid
...
+   * @see https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/rel
+   */
+  private function isValidRel($rel) : bool {
+    // List of all supported rel tags.
+    $validRelList = [
+      'alternate',
+      'author',
+      'bookmark',
+      'canonical',
+      'dns-prefetch',
+      'external',
+      'help',
+      'icon',
+      'license',
+      'manifest',
+      'modulepreload',
+      'next',
+      'nofollow',
+      'noopener',
+      'noreferrer',
+      'opener',
+      'pingback',
+      'preconnect',
+      'prefetch',
+      'preload',
+      'prerender',
+      'prev',
+      'search',
+      'shortcut icon',
+      'shortlink',
+      'stylesheet',
+      'tag',
+    ];
+    return in_array($rel, $validRelList, TRUE);

To address Dries' concerns, could the isValidRel() function be refactored a bit to move this hardcoded list to default config yaml so intrepid users could change to support the attribute values they want, and update the functions docblock so it is not tied to w3c (though our defaults would be the w3c ones)?

I'm maybe missing something here but I don't consider the w3c as infallible so I'm open to other opinions. with that said we are using the patch just to please our wcag people and get them off our backs. Our org spends a lot of money on 'wcag experts', this doesn't mean I'm in 100% agreement with them all the time.

Marking duplicate of #2922570: The node view page triggers a lot of expensive access checks for relationship links which fixes the same issue in a different way (and is/was RTBC). I've transferred issue credit from this issue to that one.