WordPress.org

Make WordPress Core

Opened 9 years ago

Closed 7 years ago

#1526 closed enhancement (fixed)

have wp-atom.php generate Atom 1.0 (fix attached)

Reported by: comatoast Owned by: markjaquith
Milestone: 2.2 Priority: normal
Severity: normal Version: 2.0.2
Component: General Keywords: bg|has-patch bg|dev-feedback
Focuses: Cc:

Description (last modified by markjaquith)

I'm not 100% sure that this doesn't have bugs of its own (site-specific things, I'd think), but I'd like to put this out there so it can get hoovered up into the next version so my changes aren't obliterated in the next upgrade...

The old version of this file (as of 1.5.1.3) generates invalid Atom, so I figured that this would be an improvement.

Attachments (12)

wp-atom.php.diff (2.8 KB) - added by comatoast 9 years ago.
the same fixes, diffed against svn.automattic.com/wordpress/branches/1.5/wp-atom.php
wp-atom.php.2.diff (3.0 KB) - added by markjaquith 9 years ago.
Patch against 1.6 SVN
wp-atom.php-patch.diff (2.8 KB) - added by markjaquith 9 years ago.
New patch by resmo
WP16-wp-atom.php.diff (2.9 KB) - added by markjaquith 9 years ago.
Patch for current WP 1.6 SVN
atom.diff (6.4 KB) - added by error 8 years ago.
Atom 1.0 support - CORRECTED
wp-atom.patch (9.9 KB) - added by yngwin 8 years ago.
new patch correcting Atom support
wp-atom-3397.patch (9.2 KB) - added by NikolasCo 8 years ago.
Updated patch, generated using dvn diff against revision 3397
wp-atom-3405.patch (15.4 KB) - added by NikolasCo 8 years ago.
Now with a sense of self
wp-atom-3406.patch (15.0 KB) - added by NikolasCo 8 years ago.
Thoroughly tested it and made quite a few corrections. Apologies for the bugspam.
wp-atom_0.3-1.0.diff (3.7 KB) - added by enricopulatzo 8 years ago.
A valid Atom 1.0 feed from the old 0.3 wp-atom.php -- tested against WP 2.0.2
wp-atom_0.3-1.0.2.diff (3.7 KB) - added by enricopulatzo 8 years ago.
Changed feed <id> element to use bloginfo('atom_url')
wp-atom-3762.patch (15.0 KB) - added by NikolasCo 8 years ago.
This makes the feed id equivalent to the self link; this seems to make sense considering the uniqueness requirement of atom:id.

Download all attachments as: .zip

Change History (39)

comatoast9 years ago

the same fixes, diffed against svn.automattic.com/wordpress/branches/1.5/wp-atom.php

markjaquith9 years ago

Patch against 1.6 SVN

comment:1 markjaquith9 years ago

  • Description modified (diff)
  • Keywords bg|has-patch bg|dev-feedback added
  • Milestone set to 1.6
  • Owner changed from anonymous to markjaquith
  • Priority changed from normal to high
  • Status changed from new to assigned
  • Version changed from 1.5.1.2 to 1.6

wp-atom.php.2.diff is a diff against current 1.6 SVN code. I have tried this out and it validates as Atom 1.0

Atom 0.3 is depreciated, so this should definitely be in the WP 1.6 release

Many thanks to comatoast for the patch

markjaquith9 years ago

New patch by resmo

markjaquith9 years ago

Patch for current WP 1.6 SVN

comment:2 markjaquith9 years ago

Latest attachment (WP16-wp-atom.php.diff) is a conversion of resmo's patch for WP 1.6

Please try it out on your 1.6 installs.

comment:3 raster9 years ago

This has the following: rss_enclosure();

I think what is needed is: atom_enclosure();

As the syntax for the two is very different.

comment:4 philor8 years ago

The Atom produced by that latest patch should be flagged as invalid by the feedvalidator within a few hours, once it updates from CVS. type="xhtml" means inline XHTML, that you are so sure your content is well-formed that you don't use any CDATA escaping. If, as is probably the case for us, you can't *guarantee* well-formedness, you have to use type="html" with CDATA escaping.

comment:5 matt8 years ago

  • Milestone changed from 1.6 to 2.1
  • Priority changed from high to low

error8 years ago

Atom 1.0 support - CORRECTED

comment:6 error8 years ago

  • Priority changed from low to normal

Hm, it seems all of the above patches were flawed in one way or another, so I've done a proper Atom 1.0 patch.

Some flaws I found: <![CDATA[...]]> used when it should not be. Categories not generated correctly or at all. Enclosures not generated correctly or at all.

This patch fixes all of these issues and validates properly.

comment:7 error8 years ago

  • Milestone changed from 2.1 to 2.0

Now that Atom 1.0 has been approved it's my opinion it should be in the next major release of WordPress, whatever number it has. :)

comment:8 matt8 years ago

  • Milestone changed from 2.0 to 2.1

comment:9 yngwin8 years ago

  • Milestone changed from 2.1 to 2.0
  • Priority changed from normal to high
  • Severity changed from enhancement to major

As Atom 0.3 is deprecated we really need to get Atom 1.0 support into the next release. This is not incredibly difficult to implement, but really necessary. Matt, if you decide once again to push this away, can you at least give an explantion why you think this is not important?

comment:10 theshaft8 years ago

I am not a developer actually, but have spent some days experimenting with my own atom feed and I would like to provide some feedback about the latest atom.diff, if that would not be a problem.
Generally, it's great, but:

  • the title entry element should really be set of type html. For example google's blogsearch displays the whole div tag as a post's title, instead of only the contents of the div tag.
  • the updated entry element contains the same timestamp as the published entry element. This also happens in the wordpress v1.5.2 atom feed. IMHO, this should display the time a post has been modified, so that bots can update their cached info. For example:
<updated><?php echo mysql2date('Y-m-d\TH:i:s\Z', $post->post_modified_gmt, false); ?></updated>
  • how about adding a link element in the entry which will have a related relation and an URL to the comments RSS feed of the post?

I am with those who say that Atom 1.0 should be implemented as soon as possible. :)

comment:11 matt8 years ago

  • Milestone changed from 2.0 to 2.1
  • Priority changed from high to low
  • Severity changed from major to enhancement

Stop changing the milestone. This will not be in 2.0.

comment:12 yngwin8 years ago

  • Priority changed from low to high
  • Severity changed from enhancement to normal

Once again, without explanation.

http://weblog.philringnalda.com/2005/12/13/your-forthcoming-feed-errors

The 0.3 format is now invalid. So will you ship WP 2.0 with an invalid Atom feed?

comment:13 matt8 years ago

  • Severity changed from normal to enhancement

Well,

  • There is no satisfactory patch available.
  • Aggregators I tried with the existing patch didn't work.
  • We have no way currently to ensure XHTML validity.
  • This is a enhancement, not a bug.
  • This is too big of a change for the 2.0 release.
  • A "priority" edit war isn't going to change any of this.

yngwin8 years ago

new patch correcting Atom support

comment:14 yngwin8 years ago

Matt: I didn't want an edit war, but I do believe this is important, and wanted an explanation from your side. Thanks for listing your reasons, finally.

I agree with you that the existing patches were lacking. As WP cannot ensure well-formed XHTML, we need to escape the content. Luckily, Atom 1.0 allows that. In the patch I have just uploaded I have corrected that. The patch works for me, and my aggregator (Sage extension in Firefox) has no problem with it. Could you please test this and consider including it into the 2.0 release if it works?

I disagree with you on the enhancement classification. How can an invalid feed not be a bug?

comment:15 error8 years ago

  • Priority changed from high to normal

It wasn't "invalid" until you all just unilaterally declared it so. Did you wipe the DTD from the face of the earth, too?

Besides, we still need comments Atom...

NikolasCo8 years ago

Updated patch, generated using dvn diff against revision 3397

comment:16 NikolasCo8 years ago

The attached patch adds my attempt to fix the alternate and self links for category feeds. It does not handle URL rewriting ("pretty permalinks") at the moment. It's not clear to me how atom feeds should be linked within that framework. I'll look into it when I have some more time.

Additionally, this patch was generated using svn diff against the current revision; it should apply cleanly.

NikolasCo8 years ago

Now with a sense of self

comment:17 NikolasCo8 years ago

The new patch includes a new (big and ugly) function: get_self_link($omit_feed = 0). It produces the "prettiest" link that returns the current resource. The $omit_feed parameter makes it suitable for generating alternate links. I haven't quite written unit tests, but the function seems to work.

So, the feeds validate and generate correct links for self and alternate. Is there anything else that needs to be addressed?

I've tried to follow code specifications and include some comments. I'd check against the Codex, but it seems to be dead at the moment.

NikolasCo8 years ago

Thoroughly tested it and made quite a few corrections. Apologies for the bugspam.

comment:18 gfmorris8 years ago

I'm just curious ... is this planned for 2.1, or is this just a placeholder milestone? I ask because I think Atom 1.0 support would be a feather in the WordPress cap. [That said, I have no illusions that it'll be trivial to get there, and I applaud Mark for taking the task on. :D]

enricopulatzo8 years ago

A valid Atom 1.0 feed from the old 0.3 wp-atom.php -- tested against WP 2.0.2

comment:19 enricopulatzo8 years ago

  • Version changed from 1.6 to 2.0.2

I've added a new patch "wp-atom_0.3-1.0.diff" that corrects the CDATA issue, but removes enclosures for now (the current rss_enclosure function isn't compatible with Atom 1.0).

enricopulatzo8 years ago

Changed feed <id> element to use bloginfo('atom_url')

comment:20 NikolasCo8 years ago

re: enricopulatzo's patch:
What's the "CDATA issue" that you were trying to address? I've noticed the following:

  • Requires all content to be well-formed XML, which can not be guaranteed (AFAIK)
  • No enclosures. Earlier patches included an atom_enclosure() function (first found in error's patch)
  • Ignores link rel="self" (my patches addressed this)

Could you please address all the problems mentioned previously in the comments here; the simplest way is to just build on other people's patches. If there's a reason you're not addressing them, an explanation would be nice.

NikolasCo8 years ago

This makes the feed id equivalent to the self link; this seems to make sense considering the uniqueness requirement of atom:id.

comment:21 enricopulatzo8 years ago

re: NikolasCo
You're 100% percent right I should be issuing patches to the previously uploaded file--sorry about that. The reason I didn't was a poor one (namely I created something that worked for me before seeing the work others had put in). My files were created prior to me checking trac and I really shouldn't have added them to the mix (at the very least not on someone else's thread.

The CDATA issue is that when you have elements that have a type="xhtml" you lose the semantic benefits by wrapping the content in a CDATA block. From my understanding of a CDATA block, data encapsulated won't necessarily mean anything to an XML interpreter. If you use type="xhtml" in Atom the content better be well-formatted XHTML.

I'll rework my offering based on what you've already added, with the exception of the CDATA part--that shouldn't be there. If people want to use XML feeds then by golly, they should be offering well-formed XML feeds (the whole "be liberal in what you accept but strict in what you offer" methodology).

comment:22 yngwin8 years ago

enrico: we must wrap the content in a CDATA block, because WordPress in no way guarantees well-formed content, and the great majority (and then I mean 99,99%) of WP users don't have the slightest clue about what well-formed X(HT)ML is!

comment:23 skeltoac8 years ago

CDATA wrapping should be conditional. A "<" in the string should be a good enough indicator.

As one who is working on a complete Atom 1.0-compatible replacement for Magpie, I see no reason for an XML parser (e.g. feed aggregator) to parse the content of a post along with the rest of the feed. If it's valid and somebody wants to parse it, they can always parse the contents of the CDATA block separately.

comment:24 C9606578 years ago

I'm no Atom expert, but I doubt that you can specify type="xhtml" and then use CDATA to hide invalid HTML. Either it is XHTML or it's not.

Assume you have a post whose body is "Peter & Mary" (without the quotes), i.e. the HTML source is "<p>Peter &amp; <b>Mary</b></p>". This can be included in an Atom feed in (at least) three ways:

type="xhtml"

<content type="xhtml">
  <div xmlns="http://www.w3.org/1999/xhtml">
    <p>Peter &amp; <b>Mary</b></p>
  </div>
</content>

(the XHTML namespace may be specified in other ways)

type="html"

<content type="html">
  &lt;p&gt;Peter &amp;amp; &lt;b&gt;Mary&lt;/b&gt;&lt;/p&gt;
</content>

type="text"

<content type="text">Peter &amp; Mary</content>

The two first methods both represent the same information. The last represents the unformatted text.

I suggest using type="html".

The type="xhtml" method is only suitable, if the body is guaranteed to be valid XHTML.

With type="text" the formatting must unnecessarily be left out. Converting HTML to a meaningful unformatted string is hard and sometimes impossible.

comment:25 c9606578 years ago

Of course you can also use CDATA instead of entity encoding for type="html" and type="text" (but not type="xhtml") like this:

<content type="html">
  <![CDATA[<p>Peter &amp; <b>Mary</b></p>]]>
</content>
<content type="text">
  <![CDATA[Peter & Mary]]>
</content>

Just don't forget to properly encode the contents of the CDATA block (necessary when the contents contain the string "]]>"). Since there is no built-in PHP function to do this CDATA escaping, I usually use entity encoding (htmlspecialchars), but both will work.

comment:26 matt7 years ago

  • Milestone changed from 2.1 to 2.2

comment:27 matt7 years ago

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

[4803] is in! :) If there are any problems with it (I'm sure there are) let's file new bugs.

Note: See TracTickets for help on using tickets.