Skip to:
Content

Opened 5 years ago

Closed 4 years ago

#1181 closed defect (duplicate)

RSS feeds not validating

Reported by: tomdebruin Owned by: chrishajer
Milestone: 1.0.3 Priority: normal
Severity: normal Version: 1.0.2
Component: Front-end Keywords: has-patch tested
Cc:

Description

There's an issue with RSS feeds not validating. The problem appears to stem from » in the titles (rss.php) so I swapped them all with ::.
It now validates. It did work fine in Safari, but Firefox and IE threw up errors.

e.g. http://validator.w3.org/feed/check.cgi?url=http%3A%2F%2Fbbpress.org%2Fforums%2Frss%2Ftopic%2Fcall-for-testers-on-the-09-branch-and-10-trunk

There's also a recommendation to not use <textInput>.

Attachments (1)

full-rss-filters.patch (5.7 KB) - added by sambauers 5 years ago.
Reverses [2374] and implements ent2ncr() via filters.

Download all attachments as: .zip

Change History (21)

comment:1 sambauers5 years ago

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

(In [2374]) Put the RSS feed description into CDATA. Fixes #1181

comment:2 sambauers5 years ago

  • Milestone set to 1.0.3

comment:3 Nightgunner55 years ago

Suggestion: Change all &raquo; to &#187; (and other non-numeric entities to their numeric counterpart)

comment:4 _ck_5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

I would highly suggest a better solution than CDATA (which is a lazy workaround at best).

CDATA looks ugly in some readers or isn't parsed.
Doesn't bbPress already have a few entity encoding routines from wordpress/backpress? Is CDATA how WordPress solves it?

sambauers5 years ago

Reverses [2374] and implements ent2ncr() via filters.

comment:5 GautamGupta4 years ago

  • Keywords has-patch needs-testing added
  • Version changed from 1.0-rc-3 (trunk) to 1.0.2

comment:6 chrishajer4 years ago

  • Owner set to chrishajer
  • Status changed from reopened to new

comment:7 chrishajer4 years ago

  • Status changed from new to assigned

comment:8 chrishajer4 years ago

What would be a good way to test this?

comment:9 GautamGupta4 years ago

When you have applied this patch on your install, then post some entities in your post text like &raquo; etc., full list can be found here (the left side values), and then check your feed from here - http://validator.w3.org/feed/

comment:10 GautamGupta4 years ago

  • Keywords tested added; needs-testing removed

comment:11 chrishajer4 years ago

Without the patch, my trunk feed validates. Is there somewhere I can see this feed as invalid? This also mentions "title" not post body.

comment:12 GautamGupta4 years ago

Did you reverse [2374] on your install?

comment:13 chrishajer4 years ago

Not explicitly, I don't think. I just applied this patch:
http://trac.bbpress.org/attachment/ticket/1181/full-rss-filters.patch

Is there more to do?

comment:15 chrishajer4 years ago

I think there are still issues with this one.

  1. CDATA is still being used in several places after applying this patch. Check here:

http://validator.w3.org/feed/check.cgi?url=http%3A%2F%2Fbbpress.chrishajer.com%2Ftrunk%2Frss%2F

If it is a lazy workaround as _ck_ suggests, why isn't it lazy to keep it in these places too? Why not remove it everywhere?

  1. Is there a good reason to leave in the <textInput>?

http://validator.w3.org/feed/docs/warning/AvoidTextInput.html

  1. I am getting some weird characters in that feed. It looks like something is converting the opening paragraph tags to
    &#60;p&#62;
    

and closing ones to

&#60;/p&#62;

I imagine if I had other HTML in the replies that the open and close (greater and less than) would be converted, incorrectly as well. Also, what about bbcode if the forum allows that? Maybe that's handled properly by being processed first and converted the html, but then I think the < and > would be converted. to 60 and 62

yeah, check this, it's a mess:

http://validator.w3.org/feed/check.cgi?url=http%3A%2F%2Fbbpress.chrishajer.com%2Ftrunk%2Frss%2Ftopic%2Ftest-time-zone-divide-time

This is not good enough yet I don't think.

comment:16 chrishajer4 years ago

We could always just remove raquo from all the php files and replace with the numeric entity. Would that resolve this ticket instead of the existing patch? I could test using html and entities in a post and see what happens.

comment:17 GautamGupta4 years ago

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

Merging feed tickets into one ticket: #1261

comment:18 chrishajer4 years ago

  • Resolution changed from duplicate to fixed

(In [2420]) RSS feed validation code changes. (You will need to update any custom theme.) Fixes #1181. Props sambauers, GautamGupta

comment:19 GautamGupta4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:20 GautamGupta4 years ago

  • Resolution set to duplicate
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.