WordPress.org

Make WordPress Core

Opened 8 months ago

Closed 8 months ago

Last modified 8 months ago

#39141 closed defect (bug) (fixed)

RSS feeds have incorrect lastBuildDate when using alternate languages

Reported by: __jester Owned by: dd32
Milestone: 4.7.1 Priority: normal
Severity: normal Version: 4.7
Component: Feeds Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

When setting my site in English, the RSS feed says:
<lastBuildDate>Wed, 30 Nov 2016 15:47:34 +0000</lastBuildDate>

But when I set it up in Dutch, it says:
<lastBuildDate>wo, 30 nov 2016 15:47:34 +0000</lastBuildDate>

This date should be a valid RFC date. Otherwise feed validators won't validate the feed.

Possible fix:

<lastBuildDate><?php
date = get_lastpostmodified( 'GMT' );
echo $date ? mysql2date( 'D, d M Y H:i:s +0000', $date, false ) : date( 'D, d M Y H:i:s +0000'
}}}</lastBuildDate>
 


Attachments (2)

39141.patch (2.3 KB) - added by stevenkword 8 months ago.
Do not translate mysql2date()
39141.unit-test.diff (2.2 KB) - added by dd32 8 months ago.

Download all attachments as: .zip

Change History (22)

#1 @swissspidy
8 months ago

Was this working before? Doesn't seem like we changed the date format here. See #38027

#2 @swissspidy
8 months ago

  • Focuses template removed

#3 @__jester
8 months ago

Yes this was working differently in 4.6.1. It happens in the mysql2date function. The date get actually gets translated if you don't set the $translate parameter to false

Last edited 8 months ago by __jester (previous) (diff)

#4 @__jester
8 months ago

In WP 4.6.1 is worked like this:

<lastBuildDate><?php echo mysql2date('D, d M Y H:i:s +0000', get_lastpostmodified('GMT'), false); ?></lastBuildDate>

#5 @swissspidy
8 months ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 4.7.1

You're right, thanks. Looks like the third parameter got lost in some files in [38925] or has always been. Seems like it has always been missing in wp-includes/feed-rss2-comments.php.

@stevenkword
8 months ago

Do not translate mysql2date()

#6 @stevenkword
8 months ago

  • Keywords has-patch needs-testing added; needs-patch removed

#7 @stevenkword
8 months ago

The translation doesn't seem to effect Atom feeds due to the 'Z' format, but I went ahead and added the parameter anyways for consistency.

Last edited 8 months ago by stevenkword (previous) (diff)

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


8 months ago

#9 @JxsDotNL
8 months ago

While this is not fixed in the core yet, you can use this code to make the RSS feeds validate again:

<?php
add_filter('date_i18n','rssfeeds_date_donttranslate',10,4);
function rssfeeds_date_donttranslate($j, $req_format, $i, $gmt){
    if(is_feed())
        return date($req_format);
    else
        return $j;
}
Last edited 8 months ago by JxsDotNL (previous) (diff)

#10 @pento
8 months ago

  • Keywords needs-unit-tests added

Unit tests would be helpful here, to avoid this bug being re-introduced in the future.

#11 @dd32
8 months ago

  • Keywords has-unit-tests added; needs-unit-tests removed

39141.unit-test.diff is an attempt at a unit test for this, unfortunately we have very little feed coverage right now. Testing the non-RSS2 feeds doesn't have much benefit either, as the date formats don't have any translated tokens.

If someone would like to extend 39141.unit-test.diff to cover the RSS2 Comments feed, please do.

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


8 months ago

#13 @dd32
8 months ago

Having taken another look at this, feed-rss2.php is the only RSS feed which actually had a date format which has any translatable components in it in the first place. Still makes sense to mark all of them as $translate = false though.

Digging into it further, the format D, d M Y H:i:s +0000 is the same as r - and as we don't need to translate it here we could probably just switch it back to that. It was changed back in r1041 (later follow up commits marked it as no-translate, this would be at least the 3rd time it's lost the no-translate flag).

#14 @dd32
8 months ago

  • Owner set to dd32
  • Resolution set to fixed
  • Status changed from new to closed

In 39613:

Feeds: Do not translate the lastBuildDate field in RSS feeds.

Props stevenkword, dd32.
Fixes #39141

#15 @dd32
8 months ago

In 39614:

Feeds: Replace the RSS2 lastBuildDate date field with the r date specifier.

See #39141.

#16 @dd32
8 months ago

In 39615:

Feeds: Do not translate the lastBuildDate field in RSS feeds.

Props stevenkword.
Partial Merge of [39613] to the 4.7 branch.
Fixes #39141.

#17 @dd32
8 months ago

  • Keywords needs-testing removed

As only the main RSS feed was affected, I only backported that specific change to 4.7.1.

For 4.8+ I've added it for all feeds, and then converted it to the r date format anyway.

#18 @dd32
8 months ago

#39394 was marked as a duplicate.

#19 @dd32
8 months ago

#39434 was marked as a duplicate.

This ticket was mentioned in Slack in #forums by clorith. View the logs.


8 months ago

Note: See TracTickets for help on using tickets.