Make WordPress Core

Opened 4 years ago

Closed 14 months ago

#47968 closed defect (bug) (fixed)

Feed (with comments) Last-Modified header only considers latest comment date

Reported by: xiven's profile xiven Owned by: audrasjb's profile audrasjb
Milestone: 6.0 Priority: normal
Severity: normal Version: 5.2.2
Component: Feeds Keywords: has-patch has-unit-tests commit assigned-for-commit
Focuses: Cc:

Description

Steps to reproduce:

  1. Create a wordpress blog
  2. Add a post
  3. Add a comment to that post
  4. Wait a while
  5. Add another post
  6. Check the Last-Modified header of the blog's feed

Expected result:
The Last-Modified header should have the date/time of the post added in step 5.

Actual result:
The Last-Modified header has the date/time of the comment added in step 3. This behaviour breaks caching and causes feed readers to believe there is no new content even when there might be new posts.

Analysis:
The fault lies in the send_headers function of class-wp.php. For a feed including comments it should use the newest date from both the get_lastcommentmodified and the get_lastpostmodified functions instead of only using the result from the get_lastcommentmodified function.

Attachments (3)

47968.patch (1.7 KB) - added by mauteri 2 years ago.
47968.1.patch (4.2 KB) - added by mauteri 22 months ago.
Updated patch with code adjustment and unit test.
47968.2.diff (5.6 KB) - added by costdev 14 months ago.
Updated patch with some tidying up. Splits tests, adds documentation, @covers annotations, $message parameters.

Download all attachments as: .zip

Change History (16)

@mauteri
2 years ago

@mauteri
22 months ago

Updated patch with code adjustment and unit test.

#1 @mauteri
22 months ago

  • Keywords has-patch has-unit-tests added

#2 @SergeyBiryukov
22 months ago

  • Milestone changed from Awaiting Review to 5.9

#3 @hellofromTonya
18 months ago

  • Milestone changed from 5.9 to 6.0

5.9 Beta 1 is happening in less than 30 minutes. I'm sorry this ticket didn't get a review and testing in time. As it's a bug that was not introduced in 5.9 cycle, moving it to 6.0 to give it the time and attention it deserves.

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


14 months ago

@costdev
14 months ago

Updated patch with some tidying up. Splits tests, adds documentation, @covers annotations, $message parameters.

#6 @audrasjb
14 months ago

The above patch looks good to me.
Added a PR to run unit tests on those changes.

#7 @audrasjb
14 months ago

  • Keywords commit assigned-for-commit added

Tests are passing, self assigning for commit.

#8 @audrasjb
14 months ago

  • Owner set to audrasjb
  • Status changed from new to assigned

#9 @audrasjb
14 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 53233:

Feeds: Use latest comment date for the Last-Modified header of comments feed.

Previously, the Last-Modified header of comments feed was using the date/time of the last comment. This behavior was breaking caching and causing feed readers to believe there is no new content even when there might be new posts.

This commit changes this behavior to use the newest date from both get_lastcommentmodified() and get_lastpostmodified() functions instead of only using the result from get_lastcommentmodified().

Props xiven, mauteri, costdev, audrasjb.
Fixes #47968.

#11 @SergeyBiryukov
14 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

There's a typo in the latest assertion: "...date of the most recent most".

#12 @audrasjb
14 months ago

ah thanks! I'll fix this right now.

#13 @audrasjb
14 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 53241:

Build/Test Tools: Typo correction in rss2 unit tests.

Follow-up to [53233].

Props SergeyBiryukov.
Fixes #47968.

Note: See TracTickets for help on using tickets.