WordPress.org

Make WordPress Core

Opened 21 months ago

Last modified 3 days ago

#46227 reopened enhancement

Add Rel-Feed Link to Header

Reported by: dshanske Owned by:
Milestone: Future Release Priority: low
Severity: trivial Version:
Component: Feeds Keywords: has-patch has-unit-tests dev-feedback
Focuses: Cc:

Description

When the front page is not the posts page, proposing that Core add a link in the header to the actual posts page, using the rel=feed designator.

https://blog.whatwg.org/feed-autodiscovery

Example

<link rel="feed" type="text/html" href="/posts" title="Posts Page" />

This would allow a system to figure out where the posts page is.

Attachments (11)

46227.diff (1.3 KB) - added by cameronamcintyre 16 months ago.
46227.2.diff (4.8 KB) - added by bogerekanzu 16 months ago.
Added the unit tests for the Add Rel link to feed in the header
46227.3.diff (4.5 KB) - added by garrett-eclipse 16 months ago.
Added @since to docblock and fixed the Unit Tests as they were failing.
46227.4.diff (3.1 KB) - added by chetan200891 16 months ago.
Refreshed patch.
46227.5.diff (1.3 KB) - added by adhitya03 15 months ago.
Move get_option( 'page_for_posts' ) into the "if"
46227.6.diff (1.3 KB) - added by adhitya03 15 months ago.
Correcting the indentation, sorry
46227.7.diff (1.3 KB) - added by adhitya03 15 months ago.
If users do not select any static page to display the posts page.
46227.8.diff (3.4 KB) - added by garrett-eclipse 15 months ago.
Reintroduce Unit Tests from 46227.4.diff that got lost. Note: These probably shouldn't live in document-title.php though.
46227.9.diff (3.0 KB) - added by adhitya03 13 months ago.
46227.10.diff (3.0 KB) - added by rebasaurus 2 weeks ago.
Updating @since to reflect 5.6
46227.10.2.diff (3.1 KB) - added by audrasjb 2 weeks ago.
patch refreshed against trunk

Download all attachments as: .zip

Change History (47)

#1 @desrosj
16 months ago

  • Keywords good-first-bug 2nd-opinion added
  • Milestone changed from Awaiting Review to Future Release

Thanks for this, @dshanske.

This seems fine to add, but I'd like to do a bit more reading. Is there any more up to date documentation? Everything I am finding is really old.

I'll mark as good-first-bug.

#3 @cameronamcintyre
16 months ago

Added new action to wp_head that checks the current page status and adds the link element if required.

#4 @dshanske
16 months ago

  • Keywords has-patch added

@bogerekanzu
16 months ago

Added the unit tests for the Add Rel link to feed in the header

#5 @bogerekanzu
16 months ago

  • Keywords has-unit-tests dev-feedback added; 2nd-opinion removed
  • Resolution set to worksforme
  • Status changed from new to closed

#6 @dshanske
16 months ago

  • Milestone changed from Future Release to 5.2.3

Shouldn't be closed till it is merged.

#7 @dshanske
16 months ago

  • Resolution worksforme deleted
  • Status changed from closed to reopened

@garrett-eclipse
16 months ago

Added @since to docblock and fixed the Unit Tests as they were failing.

#8 @garrett-eclipse
16 months ago

  • Keywords needs-testing added; good-first-bug removed

Thanks for the patch @bogerekanzu I've uploaded 46227.3.diff to fix the Unit Tests as they were failing.

I also updated the docblock for rel_feed_for_posts to include the @since and a reference to wp_head.

In the Unit Tests the dom functions were failing and some CS issues addressed as well.

Please take a look and test.
Cheers

#9 @SergeyBiryukov
16 months ago

  • Keywords needs-refresh added

46227.3.diff has some unrelated changes from #44878, needs a refresh.

@chetan200891
16 months ago

Refreshed patch.

#10 @chetan200891
16 months ago

  • Keywords needs-refresh removed

#11 @audrasjb
15 months ago

Hey there, thanks for the patch :-)

Shouldn't we check if there is something returned by get_option( 'page_for_posts' ); before displaying any bit of HTML code?

@adhitya03
15 months ago

Move get_option( 'page_for_posts' ) into the "if"

@adhitya03
15 months ago

Correcting the indentation, sorry

#12 @adhitya03
15 months ago

move get_option( 'page_for_posts' ); into the condition where it is used and ensure that get_option( 'page_for_posts' ); will always return the ID of the static page because it will run only if the posts page use static page.

#13 @audrasjb
15 months ago

Hmm, we also need to check if a blog page has been defined. That was the idea of my previous comment.

@adhitya03
15 months ago

If users do not select any static page to display the posts page.

#14 @adhitya03
15 months ago

Hi @audrasjb

I am sorry if I hard to understand your point, I this is my first contribution to the core. I have updated the patch.

@garrett-eclipse
15 months ago

Reintroduce Unit Tests from 46227.4.diff that got lost. Note: These probably shouldn't live in document-title.php though.

#15 @garrett-eclipse
15 months ago

  • Keywords 2nd-opinion reporter-feedback added
  • Milestone changed from 5.2.3 to 5.3

Hello All, Thank you for the patches to date on this.

I took a moment to read through the reference material (https://blog.whatwg.org/feed-autodiscovery) and feel we're going in the wrong direction here.

The 'feed' relationship is to indicate that the reference document is a syndication feed NOT a page.
"we have introduced a new feed relationship which indicates that the referenced document is a syndication feed. This now allows you to link to several different feeds containing different content which are not necessarily alternate versions of the page."

In WP feed <link>'s are introduced in several places with the alternate relationship attribute in many cases the issue with them is that they aren't 'necessarily alternate representations of the page' which is where the feed relationship comes in as it 'allows you to link to several different feeds containing different content which are not necessarily alternate versions of the page.'

"The feed keyword can also be used in combination with alternate to say that it is specifically the feed for the current document."

So to my understanding the existing feed <link>s should be updated to use 'rel="feed"' with the exception of the blog page itself which should use 'rel="feed alternate"' as the feed is actually an alternate representation of the page.

The existing locations which should be looked at;
Ones I feel should just be 'rel="feed"' unless they reside on the blog page;

Ones I feel should be 'rel="feed alternate"';

In no way am I a RSS/Atom Guru so would love some insight from a more experienced dev, @dshanske any thoughts?
*I very well may be wrong in which case sorry for the scare.

As well this is an enhancement and not a bug so shouldn't be on a minor milestone as it's not a reversion introduced in 5.2 so moving into 5.3 for further discussion.

I apologize to those who already submitted a patch as I had done, I should have delved deeper initially.

*Note: All quoted text comes from the reference material - https://blog.whatwg.org/feed-autodiscovery

#16 @dshanske
15 months ago

  • Keywords reporter-feedback removed

Your statement is correct. However, I only scoped this ticket to adding the link for rel-feed when the front page of the site is not the posts page, as this adds a defined benefit we don't have now...allowing for feed discovery in a not unpopular case.

Suggesting the second issue...adding it to rel-alternate situations, should be a separate ticket.

#17 @garrett-eclipse
15 months ago

Thanks @dshanske I appreciate you feeding back there.

So I understand a bit more the issue here and make sure I'm on the same page...

In a default WP install with a Homepage, which isn't the posts page, that homepage already has a link to the feed - <link rel="alternate" type="application/rss+xml" title="Rel Feed » Feed" href="http://rel-feed.local/feed/">. This is a rel="alternate" link but should instead be updated to a rel="feed" throughout the site except when it's on the posts page where it should be a rel="feed alternate".

So this request would be to take this line - https://github.com/WordPress/WordPress/blob/master/wp-includes/general-template.php#L2835
And update it as follows;
echo '<link rel="feed' . ( is_home() ? ' alternate' : '' ) . '" type="' . feed_content_type() . '" title="' . esc_attr( sprintf( $args['feedtitle'], get_bloginfo( 'name' ), $args['separator'] ) ) . '" href="' . esc_url( get_feed_link() ) . "\" />\n";
*Which changes that feed link from alternate to feed for all pages and then appends alternate if it's on the actual blog page.

Am I correct in that assumption? Just want to be sure before heading down that path.
Thanks

#18 @dshanske
15 months ago

Actually no. I was proposing a link to the html feed page, not the rss one.

#19 @dshanske
15 months ago

You are correct about the application of rel-feed otherwise, by the way. Although my concern in removing the alternate is that it has been there for so long that it would break RSS readers expecting it.

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


13 months ago

#21 @johnbillion
13 months ago

46227.8.diff is missing the call to add_action as seen in 46227.7.diff .

#22 @davidbaumwald
13 months ago

  • Milestone changed from 5.3 to Future Release

This enhancement needs a direction an implementation. With version 5.3 Beta 1 releasing tomorrow, this is being moved to ‘Future Release’.

@adhitya03
13 months ago

#23 @adhitya03
13 months ago

  1. Add missing add_action in 46227.8.diff
  2. Delete function rel_feed_for_posts() that shows three times in 46227.8.diff

#24 @dshanske
12 months ago

  • Keywords needs-testing 2nd-opinion removed
  • Milestone changed from Future Release to 5.4

I think the latest iteration accomplishes the goal of linking to the posts page explicitly when the home page is not also the posts page. This has been a significant problem in finding where you can get a HTML page with all the posts on a site.

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


9 months ago

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


8 months ago

#27 @davidbaumwald
8 months ago

  • Milestone changed from 5.4 to Future Release

This ticket still needs a review, and with 5.4 Beta 1 landing tomorrow, this is being moved to Future Release. If any maintainer or committer feels this can be included in 5.4 or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

#28 @dshanske
5 months ago

  • Milestone changed from Future Release to 5.5

#29 @davidbaumwald
4 months ago

  • Milestone changed from 5.5 to Future Release

With 5.5 Beta 1 releasing tomorrow, this is being moved to Future Release. If any maintainer or committer feels this can be resolved in to, or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

#30 @dshanske
2 months ago

  • Milestone changed from Future Release to 5.6

This ticket was mentioned in Slack in #core by metalandcoffee. View the logs.


2 weeks ago

@rebasaurus
2 weeks ago

Updating @since to reflect 5.6

#32 @hellofromTonya
2 weeks ago

5.6 beta 1 is in less than 2 weeks. @stevenkword Is this ticket on your radar to land in the 5.6 milestone? If yes, what's next to move it forward? If no, is it okay to punt?

@audrasjb
2 weeks ago

patch refreshed against trunk

#33 @audrasjb
2 weeks ago

@rebasaurus thanks for the refresh! Though we prefer to use "5.6.0" than "5.6" :)

#34 @dshanske
2 weeks ago

Again, summarizing for a dev-note. "In WordPress 5.6, when the posts page of your site is not the front page, a non-visible link will be added to the page, marked as rel='feed', type="text/html' to allow the posts page to be found."

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


3 days ago

#36 @davidbaumwald
3 days ago

  • Milestone changed from 5.6 to Future Release

With 5.6 Beta 1 releasing tomorrow and this being a low priority, this is being moved to Future Release. If any maintainer or committer feels this can be resolved in time, or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

Note: See TracTickets for help on using tickets.