RSS feed icon should not include contextual filters arguments in link if the feed display does not support contextual filters [#1278760]

Hi,

I have a view with 2 page displays (blog, and blog/) and 1 feed display (feed/blog). I am attaching the feed to both the displays.

The page_2 display (blog/) accepts a contextual filter for year, and will show a list of blogs filtered by the given year.

The rss icon for page_1 (blog) links to 'feed/blog', which is correct. The rss icon for page_2 (blog/2011), however, shows 'feed/blog/2011', which is incorrect, as my feed does not have a contextual filter.

Though the feed on feed/blog/2011 does show the same content as feed/blog, for SEO purposes, they should ideally use the same url if they have the same content.

I have created a patch to take into account whether the feed display supports arguments, and if it does, the arguments will be added to the path (as it currently does), if not, the arguments will be excluded from the path.

Hope this helps.

Cheers,
Victoria

Comment	File	Size	Author
#7	views-1278760-rss-arguments.patch	734 bytes	ygerasimov
#7
#5	views-feed-contextual-filters-1278760.patch	943 bytes	jamix
#5
#4	views-feed-contextual-filters-1278760.patch	942 bytes	jamix
#4
#1	1278760-rss-icon-link.patch.1.txt	926 bytes	victoriachan

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

victoriachan CreditAttribution: victoriachan commented 14 September 2011 at 10:06

File	Size
1278760-rss-icon-link.patch.1.txt	926 bytes

Attaching patch. It is only a few lines:


class views_plugin_style_rss extends views_plugin_style {
  function attach_to($display_id, $path, $title) {
// ...

// Don't add arguments to RSS path if the feed does not support arguments
$feed_path = $this->display->display_options['arguments'] ? $this->view->get_url(NULL, $path) : $path;
$url = url($feed_path, $url_options);

//...

Comment #2

dawehner

German

CreditAttribution: dawehner commented 27 September 2011 at 21:42

Status:

Needs review

» Needs work

+++ plugins/views_plugin_style_rss.incundefined
@@ -14,12 +14,16 @@ class views_plugin_style_rss extends views_plugin_style {
+    $feed_path = $this->display->display_options['arguments'] ? $this->view->get_url(NULL, $path) : $path;

It's probably better to use either $view->argument or $view->get_items because $this->display->display_options might have problems with overriden values etc.

Comment #3

jamix CreditAttribution: jamix commented 10 August 2012 at 12:37

#1 works great here, thanks.

@dawehner: $this->view seems to be holding the object for the display the feed is being attached to, which makes relying on $this->view->argument not work.

Comment #4

jamix CreditAttribution: jamix commented 10 August 2012 at 12:49

File	Size
views-feed-contextual-filters-1278760.patch	942 bytes

Re-rolling patch from #1 to fix PHP notice when there are no contextual filters defined in the view at all.

Comment #5

jamix CreditAttribution: jamix commented 10 August 2012 at 12:54

File	Size
views-feed-contextual-filters-1278760.patch	943 bytes

Attaching the correct patch file for #4.

Comment #6

MKorostoff CreditAttribution: MKorostoff commented 10 August 2012 at 13:20

Thanks

Comment #7

ygerasimov CreditAttribution: ygerasimov commented 11 August 2012 at 13:40

Version:	7.x-3.0-rc1	» 7.x-3.x-dev
Status:	Needs work	» Needs review

File	Size
views-1278760-rss-arguments.patch	734 bytes

Rerolled #5 patch without trailing spaces.

Comment #8

jos_s CreditAttribution: jos_s commented 22 July 2015 at 12:27

Issue summary:

View changes

#7 worked for me. Would be nice if this patch could now be included in Views. Or is anything stopping this?

Comment #9

22 July 2015 at 12:28

jos_s queued 7: views-1278760-rss-arguments.patch for re-testing.

Comment #10

Anonymous (not verified) CreditAttribution: Anonymous commented 4 February 2016 at 23:56

#7 worked for me well, using Views 7.x-3.13 and Drupal 7.42.

This patch is really useful - nobody wants people to subscribe to a feed with last year's blog entries, news etc. Like jos_s (#8) I'd be very happy to see the patch included in Views.

Comment #11

ron_s CreditAttribution: ron_s commented 24 September 2016 at 22:41

Status:	Needs review	» Needs work
Related issues:		+#2791847: Don't add arguments to ical path if the feed does not support arguments, +#1722348: Contextual filters should not be added to feed URL if the feed display doesn't support them

Traditionally, Views RSS and Date iCal feeds work by inheriting the contextual filters from the Views displays to which they are attached. There may not be any filters on existing feeds, and patch #7 will break feeds that have paths such as "/calendar/%/rss.xml" and no filter.

Something needs to be included in the patch to notify developers that existing feeds may stop working. There also needs to be coordination with patches for Views RSS (https://www.drupal.org/node/1722348) and Date iCal (https://www.drupal.org/node/2791847).

As well, I understand the issue being recognized, but have the implications of no longer inheriting contextual filters for feeds been fully thought through? Thanks.