r61026 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r61025‎ | r61026 | r61027 >
Date:21:29, 13 January 2010
Author:simetrical
Status:ok (Comments)
Tags:
Comment:
Only advertise RSS feeds by default, not Atom

Advertising both is pointless, since every feed reader in the universe
supports both. Their functionality is essentially identical. And users
might be given a choice of multiple feeds to read when there's really
only one distinct feed. Plus, it cuts a line from the <head>. The Atom
feed still exists, and you can get the <head> link back if you really
want it with $wgAdvertisedFeedTypes.
Modified paths:
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/includes/DefaultSettings.php (modified) (history)
  • /trunk/phase3/includes/OutputPage.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/DefaultSettings.php
@@ -2946,6 +2946,12 @@
29472947 $wgOverrideSiteFeed = array();
29482948
29492949 /**
 2950+ * Which feed types should we provide by default? This can include 'rss',
 2951+ * 'atom', neither, or both.
 2952+ */
 2953+$wgAdvertisedFeedTypes = array( 'rss' );
 2954+
 2955+/**
29502956 * Additional namespaces. If the namespaces defined in Language.php and
29512957 * Namespace.php are insufficient, you can create new ones here, for example,
29522958 * to import Help files in other languages.
Index: trunk/phase3/includes/OutputPage.php
@@ -402,14 +402,15 @@
403403 }
404404
405405 public function setFeedAppendQuery( $val ) {
406 - global $wgFeedClasses;
 406+ global $wgFeedClasses, $wgAdvertisedFeedTypes;
407407
408408 $this->mFeedLinks = array();
409409
410 - foreach( $wgFeedClasses as $type => $class ) {
 410+ foreach ( $wgAdvertisedFeedTypes as $type ) {
411411 $query = "feed=$type";
412 - if ( is_string( $val ) )
 412+ if ( is_string( $val ) ) {
413413 $query .= '&' . $val;
 414+ }
414415 $this->mFeedLinks[$type] = $this->getTitle()->getLocalURL( $query );
415416 }
416417 }
@@ -1779,7 +1780,8 @@
17801781 $tags[] = $this->feedLink(
17811782 $format,
17821783 $link,
1783 - wfMsg( "page-{$format}-feed", $this->getTitle()->getPrefixedText() ) ); # Used messages: 'page-rss-feed' and 'page-atom-feed' (for an easier grep)
 1784+ # Used messages: 'page-rss-feed' and 'page-atom-feed' (for an easier grep)
 1785+ wfMsg( "page-{$format}-feed", $this->getTitle()->getPrefixedText() ) );
17841786 }
17851787
17861788 # Recent changes feed should appear on every page (except recentchanges,
@@ -1790,7 +1792,7 @@
17911793 # or "Breaking news" one). For this, we see if $wgOverrideSiteFeed is defined.
17921794 # If so, use it instead.
17931795
1794 - global $wgOverrideSiteFeed, $wgSitename, $wgFeedClasses;
 1796+ global $wgOverrideSiteFeed, $wgSitename, $wgFeedClasses, $wgAdvertisedFeedTypes;
17951797 $rctitle = SpecialPage::getTitleFor( 'Recentchanges' );
17961798
17971799 if ( $wgOverrideSiteFeed ) {
@@ -1800,9 +1802,8 @@
18011803 htmlspecialchars( $feedUrl ),
18021804 wfMsg( "site-{$type}-feed", $wgSitename ) );
18031805 }
1804 - }
1805 - else if ( $this->getTitle()->getPrefixedText() != $rctitle->getPrefixedText() ) {
1806 - foreach( $wgFeedClasses as $format => $class ) {
 1806+ } elseif ( $this->getTitle()->getPrefixedText() != $rctitle->getPrefixedText() ) {
 1807+ foreach ( $wgAdvertisedFeedTypes as $format ) {
18071808 $tags[] = $this->feedLink(
18081809 $format,
18091810 $rctitle->getLocalURL( "feed={$format}" ),
Index: trunk/phase3/RELEASE-NOTES
@@ -91,6 +91,9 @@
9292 If not specified it will default to $wgScriptPath/extensions
9393 * Added $wgCountTotalSearchHits to make search UI display total number of hits
9494 with some search engines.
 95+* Added $wgAdvertisedFeedTypes to decide what feed types (RSS, Atom, both, or
 96+ neither) MediaWiki advertises. Default is array( 'rss' ), so Atom is no
 97+ longer advertised by default (but it still works).
9598
9699
97100 === New features in 1.16 ===

Follow-up revisions

RevisionCommit summaryAuthorDate
r61037follow up r61026 - use Atom by defaultmah01:31, 14 January 2010

Comments

#Comment by MarkAHershberger (talk | contribs)   23:53, 13 January 2010

I would prefer Atom to RSS since there seems to be more encoding problems (on the client side) with RSS and Atom made efforts to address that.

#Comment by Simetrical (talk | contribs)   00:48, 14 January 2010

I don't have a strong opinion here, RSS is just older and more recognizable. What kind of encoding problems? Has anyone filed a bug report with us on it?

#Comment by MarkAHershberger (talk | contribs)   01:06, 14 January 2010

http://www.xn--8ws00zhy3a.com/blog/2006/06/encoding-rss-titles -- note the chart that shows which readers support double encoding and which support single encoding. It is old (2006) but this was one of the motivators for the development of Atom -- better specification, less ad hoc. (I've seen encoding problems in the past year in Google Reader for RSS feeds, so the problem hasn't gone away.)

Note that this is mostly a problem of a poor spec. Yes, RSS is older, but, at least in Atom people can't complain that there are no clear rules about encoding.

#Comment by Simetrical (talk | contribs)   01:23, 14 January 2010

Well, it's a one-line change, feel free to make it if you have an opinion. I don't care either way.

Status & tagging log