Remove {{ feed_icons }} from page template (page.html.twig)

Also if someone really wanted the automatic feature, it could be done using js - that way you have access to the final<head> output.

Very, very, very few people ever use it, since browsers make it easy to know there's a feed to subscribe to on the current page, since there's of course still the
tag.

I am sorry but this statement is 100% wrong. Have a look at the following URL with your browser: https://www.drupal.org/node
Neither chrome nor firefox helps you to figure out that there is a feed. Yes, it used to be like that, but this is no longer the case. Showing people that you have an RSS feed available is a thing for blogs.

And note: views has probably just no test coverage but renders out the icon by default on /node

@dawehner this wouldn't get rid of the Views rendering (or at least it shouldn't).

I agree that there is no need of having a RSS attached to every single Drupal page. It's a better solution to provide it if you really need it using the Syndicate block.

Patch looks pretty and also provided screenshots:

BEFORE

AFTER

All other points still stand, and further reduce the problematicness of your (valid!) point: the "Syndicate" block still exists, and Views has the ability to show a feed icon as well.

Looking at #2357937-11: Remove {{ feed_icons }} from page template (page.html.twig) views seem to have lost the capability to render it, didn't it :)

Does it make sense then to add the js code to add access to the final head output? That would satisfy both @wim and @dawehner's concerns.

Yep, good call, lets get rid of it. +1 for the reasons stated in the OP.

+1, if people do use RSS then they don't use it like this.

Does this mean the front page view needs configuring the show the RSS icon though?

I am sorry but this statement is 100% wrong. Have a look at the following URL with your browser: https://www.drupal.org/node
Neither chrome nor firefox helps you to figure out that there is a feed. Yes, it used to be like that, but this is no longer the case. Showing people that you have an RSS feed available is a thing for blogs.

They don't because they removed support for it. If you care enough then you can download an RSS plugin. Here it is in action:

IMHO it doesn't. It's antiquated to show it at all, as I mentioned in #9 — do you know of any prominent sites still showing the RSS feed icon? As LewisNyman showed in #18, people using feeds can easily make

Well yeah I think power users would use some browser plugin anyway.

For blogs using the syndicate block is probably a good idea.

@Wim Leers
We need to update the Rss.php

Why? Nothing in this patch affects that code? So are you saying we should make it more consistent? Or fix a bug? Or…?

Just to be sure, I try to be nice again ;)

The current code looks like the following:

    $url = _url($this->view->getUrl(NULL, $path), $url_options);
    if ($display->hasPath()) {
      if (empty($this->preview)) {
        // Add a call for drupal_add_feed to the view attached data.
        $build['#attached']['drupal_add_feed'][] = array($url, $title);
      }
    }
    else {
      $feed_icon = array(
        '#theme' => 'feed_icon',
        '#url' => $url,
        '#title' => $title,
      );
      $this->view->feed_icon = $feed_icon;

      // Add a call for drupal_add_html_head_link to the view attached data.
      $build['#attached']['drupal_add_html_head_link'][][] = array(
        'rel' => 'alternate',
        'type' => 'application/rss+xml',
        'title' => $title,
        'href' => $url,
      );
    }
  }

So for page displays we use drupal_add_feed() which allows us to get rid of adding doing a lot of stuff. For blocks we can't that easy render the feed icon, so
we have to drop more code, as well as get rid of $view->feed_icon, I guess ...

Just to be clear, we would have to expand the capabilities of the syndicate block to make it actually useful.

Just from "prominent sites" argument using the icon, when I'm testing the RSS aggregator I always go to news sites:
BBC

NBC

CBC

I do agree with removing it so big +1 to this issue in general. And RSS is getting used much less than it was(Damn you twitter!)

Just want to echo @dawehner's concerns and hope we don't rush to hastily to remove a feature without providing a DX friendly alternative.

What's the plan to make it fairly easy to add tags/elements to head from blocks, views, wherever? #attached is covering js/css well enough at the moment. Specifically it looks like _drupal_add_html_head_link is a deprecated function, so were is that headed?

Feed icon should be fine removed as long as we can get the feed_url to the template a themer can wrap it around their SVG, png, icon font, or whatever. Which should be really easy on a Views template id' expect. Getting rid of view->feed_icon should be fine to do as long as feed_url is discoverable.

To answer my own question, as I wasn't aware...
Change record (updated because there was a typo for feed):
https://www.drupal.org/node/2160069

Couple of questions on the current patch:

1. +++ b/core/includes/common.inc
   @@ -234,39 +234,6 @@ function drupal_get_html_head($render = TRUE) {
   -function _drupal_add_feed($url = NULL, $title = '') {
   -  $stored_feed_links = &drupal_static(__FUNCTION__, array());
   

   Why a static here and is it still needed in drupal_process_attached now that the code has moved there?

2. +++ b/core/includes/common.inc
   @@ -1872,7 +1839,13 @@ function drupal_process_attached($elements, $dependency_check = FALSE) {
   +          $args = [[
   +            'href' => $args[0],
   +            'rel' => 'alternate',
   +            'title' => $args[1],
   +            'type' => 'application/rss+xml',
   +          ]];
   +          call_user_func_array('_drupal_add_html_head_link', $args);
   

   Doesn't this make it difficult to change the type if it's an opml feed or something?

Removing this special snowflake variable has been in my head for months now. Thanks for taking care of this Wim.

I used to use RSS all the time. I can't remember the last time I used it. As I grow more disenchanted with Twitter, I think about returning to RSS. But core should be solving the 80% use cases, and I'm not sure having a RSS link visible on the page falls into that 80% use case anymore.

+1 to saying goodbye to the feeds variable.

+1 to remove in favour of block

IMO most of sites needs separate block with own links so RSS is mostly an icon within others in "social" block and I would like to see follow-up to transform "syndicate" block into a kind of "follow" block.

@dawehner +1 to

For blocks we can't that easy render the feed icon, so
we have to drop more code, as well as get rid of $view->feed_icon, I guess ...

Only page views should be able to expose head link.

@Wim Leers
Sure, we can remove it, but we should be aware of the fact that the syndicate block is that helpful for the day to day use.
It simply points to '/rss.xml' which is one URL controlled by views. People might easily build other rss feeds, so we maybe should expand that block a bit.

#27 suppose it's not good idea to allow a block to get page state...

I think core's standard profile could ship placed syndicate block into main content region to emulate BC for taxonomy pages at least. And only one nitpick:

+++ b/core/modules/views/src/Plugin/views/style/Rss.php
@@ -42,29 +41,22 @@ public function attachTo(array &$build, $display_id, $path, $title) {
+    $feed_icon = array(
+      '#theme' => 'feed_icon',
...
+    $this->view->feed_icon = $feed_icon;

$feed_icon is useless variable now

Reviewed this, looks good to me. dawehner's concerns appear to have been addressed, so I think this is all set.

Regarding the syndicate/follow block, I'm sure contrib could take care of that or if we need them they could be in a followup. I'm good with the RTBC, this is a nice cruft remover:)

Re:

#24.1: I don't understand the question.

I was just curious what that static cache was speeding up and if that "speed" was needed somewhere else in the patch.
I'm guessing it is no longer needed as we aren't _drupal_add_feeds'ing just adding another link to the head.

and #24.2 I see what you're doing now, keeping the API in tact is all... if someone were to want to do opml or something they can just build a ['#attached']['html_head_link'][]

Not marking down from RTBC, but aren't the screenshots in #23 advocating for *keeping* the feed icons on the page (either through Views or through a More Betterer Syndicate block)? No offense, but I'm not sure a bunch of Drupal core developers are a very representative sample of website users as a whole. ;) The fact that BBC et al keep one or more feed icons around rather than use that precious real estate as ad space seems to imply it's widely used by the internet at large (though as a counter-point I wasn't able to find such an icon on CNN.com, which AFAIK is the largest news website).

Anyway, rather than get into that whole debate, moving the RSS feed rendering to Views seems to address both issues in the OP (it doesn't futz with page rendering, and you can get rid of it if you want). So why are we trying to do this first, versus postponing on #2359161: Feed icons missing in views blocks and pages?

Given this blocks #2352155: Remove HtmlFragment/HtmlPage, bumping to critical.

Leaving RTBC a bit longer in case there's more to discuss, but personally I agree fixing the Views regression from 7.x is a separate issue to sorting out the chicken and egg page variable.

This issue needs a change record because it

removes a "feature" that is 10 years old, has a crappy UX, has a crappy DX, and has a crappy TX.

I guess the change record should be linked to the views issue to and once that is fixed contain instructions on how to build a replacement.

Thanks for the change notice.

Committed/pushed to 8.0.x, thanks!

If we want to bump #2359161: Feed icons missing in views blocks and pages to critical we should, I think it's major so it depends how much the missing feature is considered a regression (and whether that regression is worth holding the release up for).

afaik, that issue will not fix the now missing icons for pages? So it's not about fixing an existing regression, but creating a new one as well.

So this is knowingly creating bugs to help solve other bugs? :)

Ok, then I missed something here. Thanks!

published CR