WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 7 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 7 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)

comment:1 shorty1147 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?

comment:2 shorty1147 years ago

  • Keywords bg|needs-patch removed

comment:3 shorty1147 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?

comment:4 follow-ups: matt7 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.

comment:5 markjaquith7 years ago

  • Milestone 2.0.6 deleted

comment:6 in reply to: ↑ 4 link927 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.

comment:7 in reply to: ↑ 4 link927 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.

comment:8 in reply to: ↑ 4 link927 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.

comment:9 link927 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).

comment:10 link927 years ago

  • Milestone set to 2.1

link927 years ago

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

comment:11 link927 years ago

  • Keywords has-patch added

comment:12 matt7 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.

comment:13 markjaquith7 years ago

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

comment:14 foolswisdom7 years ago

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

comment:15 markjaquith7 years ago

  • Milestone changed from 2.0.7 to 2.0.8

Let's let this sit in 2.1 for a bit

comment:16 link927 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".

comment:17 link927 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.

comment:18 foolswisdom7 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.

comment:19 link927 years ago

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

comment:20 markjaquith7 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

comment:21 Nazgul7 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.

comment:22 foolswisdom7 years ago

  • Milestone changed from 2.0.10 to 2.0.11

comment:23 link927 years ago

Surely this has sat in 2.1 for long enough?

comment:24 foolswisdom7 years ago

  • Milestone changed from 2.0.11 to 2.0.eventually

comment:25 rubys7 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.

comment:26 foolswisdom7 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.

comment:27 rubys7 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.

comment:28 markjaquith7 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.