WordPress.org

Make WordPress Core

Opened 9 years ago

Closed 8 years ago

#3377 closed defect (bug) (fixed)

Atom feed contradicts itself

Reported by: link92 Owned by: markjaquith
Milestone: 2.1 Priority: low
Severity: normal Version: 2.0.5
Component: General Keywords: has-patch
Focuses: Cc:

Description

<summary type="text/plain" mode="escaped">

From a parser POV, what does this mean? Normally you only escape the content when you have HTML content, yet what we're given in the type attribute says it's text/plain. Since both are MAY [RFC2119] in Atom 0.3, many (if not all parsers) will just take one attribute and ignore the other. Fx 2.0 just treats it as unescaped plain text.

Attachments (1)

wp-atom.php.patch (795 bytes) - added by link92 9 years ago.
Change the MIME type of the <summary> to bloginfo('html_type') - patch should apply against 2.1 and 2.0

Download all attachments as: .zip

Change History (29)

#1 @shorty114
9 years ago

  • Keywords bg|dev-feedback bg|2nd-opinion bg|needs-patch added
  • Milestone set to 2.0.6
  • Priority changed from normal to low
  • Version set to 2.0.5

It would make sense that once it's escaped, it can be used as text/plain. But maybe do type="text/html" instead?

#2 @shorty114
9 years ago

  • Keywords bg|needs-patch removed

#3 @shorty114
9 years ago

  • Owner changed from anonymous to shorty114
  • Status changed from new to assigned

I would be glad to do a patch but any devs have an opinion?

#4 follow-ups: @matt
9 years ago

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

Since our feeds currently work everywhere, I'm not inclined to change this.

#5 @markjaquith
9 years ago

  • Milestone 2.0.6 deleted

#6 in reply to: ↑ 4 @link92
9 years ago

Replying to matt:

Since our feeds currently work everywhere, I'm not inclined to change this.

Huh? As I said in the report, it breaks in Fx 2.0.

#7 in reply to: ↑ 4 @link92
9 years ago

<summary type="text/plain" mode="escaped"><![CDATA[This &amp;amp; this]]></summary>

Will come out of almost every parser as meaning a summary with contents of "This &amp;amp; this" which is plain text.

<summary type="text/plain" mode="escaped">This &amp;amp; this</summary>

Will come out of almost every parser as meaning a summary with contents of "This &amp; this" which is plain text.

The real issue is that we have text content which is escaped (which under the Atom 0.3 spec means nothing more than just follow the normal XML rules (eg. remove entities in PCDATA, but not CDATA)), and that it is plain text and therefore the data is the raw content after being escaped. The two options for this are to:

  1. Remove the CDATA section, so the entities are decoded by the parser,
  2. Change the MIME type to text/html, so it's parsed by an HTML parser.

#8 in reply to: ↑ 4 @link92
9 years ago

Replying to matt:

Since our feeds currently work everywhere, I'm not inclined to change this.

Also, what feed parsers does it work in (most feed parsers just show the content, not the summary, so it's futile naming any like that)? Anything that it works in is horribly broken.

#9 @link92
9 years ago

  • Resolution wontfix deleted
  • Status changed from closed to reopened

I'm reopening this on grounds that nobody has questioned what I said above (and that therefore it still exists).

#10 @link92
9 years ago

  • Milestone set to 2.1

@link92
9 years ago

Change the MIME type of the <summary> to bloginfo('html_type') - patch should apply against 2.1 and 2.0

#11 @link92
9 years ago

  • Keywords has-patch added

#12 @matt
9 years ago

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

(In [4688]) Summaries in Atom should properly describe their content, fixes #3377. Hat tip: link92.

#13 @markjaquith
9 years ago

  • Milestone changed from 2.1 to 2.0.7
  • Resolution fixed deleted
  • Status changed from closed to reopened

#14 @foolswisdom
9 years ago

  • Keywords bg|dev-feedback bg|2nd-opinion removed
  • Owner changed from shorty114 to markjaquith
  • Status changed from reopened to new

#15 @markjaquith
9 years ago

  • Milestone changed from 2.0.7 to 2.0.8

Let's let this sit in 2.1 for a bit

#16 @link92
9 years ago

This is now broken again in trunk: <summary> isn't allowed to have a MIME type on @type (in Atom 1.0 only <atom:content>'s @type allows a MIME type). The best thing to do is probably just hardcode @type="html".

#17 @link92
9 years ago

I might add that due to it being a MIME type, it now defaults to "text", which brings us back to where this bug started.

#18 @foolswisdom
9 years ago

link92, the patch above if applied for 2.0.8 would not have the problems you describe in the two comments above? 2.1 currently is broken in this way or only trunk?

For my brains sake would it makes sense to split off new bugs based on the modified implimentation in 2.1 or trunk? Please.

#19 @link92
9 years ago

2.1 fixes this bug. 2.0 still has this bug. 2.2 has the problems in my last two comments.

#20 @markjaquith
9 years ago

  • Milestone changed from 2.0.8 to 2.0.9

link92, please open a separate ticket for the issue in 2.2 (which now has Atom 1.0)

2.1 just came out, so this needs to sit there until 2.0.9

#21 @Nazgul
9 years ago

  • Milestone changed from 2.0.9 to 2.0.10

2.0.9 is out, so bumping the milestone to 2.0.10.

#22 @foolswisdom
9 years ago

  • Milestone changed from 2.0.10 to 2.0.11

#23 @link92
9 years ago

Surely this has sat in 2.1 for long enough?

#24 @foolswisdom
9 years ago

  • Milestone changed from 2.0.11 to 2.0.eventually

#25 @rubys
8 years ago

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

This defect was fixed in WordPress 2.2, as a byproduct of supporting Atom 1.0.

#26 @foolswisdom
8 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Re-opening as this ticket was/is targetted at the long term maintenance 2.0.x branch.

#27 @rubys
8 years ago

re: re-opening, fair enough; though as one of the authors of that spec, the problem isn't that Wordpress produced an invalid feed, it is that that version of the spec itself was ambiguous.

#28 @markjaquith
8 years ago

  • Milestone changed from 2.0.eventually to 2.1
  • Resolution set to fixed
  • Status changed from reopened to closed

In that case, we have even less reason to change this. It's fairly hard for us to get changes like this into the 2.0.x branch because of pressure from the Debian side to limit it to security/stability issues.

Note: See TracTickets for help on using tickets.