Aggregator titles display quotes and other characters with HTML entity equivalents badly (write tests)

I can't properly display the html entities in the comments. It's all screwed up. Let me try with spaces before the semicolon. You should mentally remove them.

gets converted to

which results in the following output:

> Most feed fields contain regular, unmarked up text.

Well, Google uses htmlentities. They are a pretty major supplier of feeds.

Does anybody have brainstorming ideas of how to work around this? I'd settle for just dropping the entities from the RSS feed, which would be better than the current mash of ascii characters on the front page of the site.....

I downloaded newest version of 4.7.3 aggregator.module, and it still has the problem. I has to modify the file in multiple locations (using advise for filter_xss) to get it to work both in the block AND the "more" summary page... just a heads up. http://wellweight.org

Patch for 4.7 cvs.

Will this patch be part of drupal 5.0 core?
I am running drupal 5 CVS on a test site, and I still see & # 3 9 ; (spaces inserted there to prevent it from being parsed)

Releasing this issue back into the wild. I'm not sure what's the best way to fix this..

I agree 100%

No longer applies.

What is the status of this problem? I use Google news and get this ugly output as well.

What is the valid standard in XML - " or " or & #39;? I don't know how much more attention people pay to feed validity than website validity - would it be enough to require validity and reject quirky feeds, or do we lose a lot of functionality?

Edit: I note that whatever filter is here on d.o. is one whose code I would probably yell upon seeing. You do not unescape & amp; # 39; twice to make '. You just don't.

I implemented the changes suggested by ahoeben above in his patch in post #11. Then I tried changing all the check_plain references to aggregator_filter_xss, but still got funky characters in Google News feeds on my site .

ahoeben's patch seems to clean titles that show up in feed blocks, but not in the Drupal aggregator category view which was still giving me funky single quotes in feed titles. It looks like the funky symbols are still being stored in the database, and ahoeben's patch fixes them when they are displayed in blocks.

Since the funky stuff was still getting displayed I wanted to remove them before they go into the database & thus make ahoeben's display patch unnecessary. So I added htmlspecialchars_decode() in the aggregator.module aggregator_parse_feed function.

It works great, whenever a "& #39;" shows up in the title of my news feeds, it's replaced with a single quote and then inserted into the database for later use.

Here's the line before:

$title = $item['TITLE'];

Here it is after:

$title = htmlspecialchars_decode($item['TITLE'], ENT_QUOTES);

For me on 6.3 it's as the above post. I have a google news feed displayed in a block on a site, and the title links were showing &blah; style text - much in the same way that Firefox RSS feed reader does for sites like Slashdot et all interestingly.

Anyway - all I had to do was as jdefay says:

add htmlspecialchars_decode() in the aggregator.module aggregator_parse_feed function.

Here's the line before:

$title = $item['TITLE'];

Here it is after:

$title = htmlspecialchars_decode($item['TITLE'], ENT_QUOTES);

jdefay rox.

D

I just noticed that the htmlspecialchars_decode() function is only available in PHP >= 5.1.0. I am using PHP 4.4.4 which is supported by Drupal 6.3. Unfortunately I am unable to upgrade PHP because my site is hostsed on a server managed by my ISP and uses cpanel. So, the version of PHP is out of my control.

Is there a PHP 4.4.4 function which will do same/similar, or do I need to suffer this until PHP 5.1.0 or later is available ?

Thanks,
Tim

This bug still exists as of D6.6 in core aggregator.module. Any plans for a patch to be brought into core?

--Brad

I can confirm that @jdefay's patch (#26) works well for >PHP5; I've been using it for months. I have not tested the other version, nor can I vouch for whether or not doing this is 'the Drupal way.' (I just know that my users need the feeds to work properly!)

jdefay,

I just looked at your site, which has a Google News Feed here: http://jason.defay.org/aggregator/categories (titled 'UCSD In the News').

How did you get that to work? When I setup something similar, all the ampersands (&) are stripped out of the links, which breaks them. Example:

http://news.google.com/news/url?sa=T&ct=us/0-0&fd=R&url=http://www3.sign...

http://news.google.com/news/url?sa=Tct=us/0-0fd=Rurl=http://www3.signons...

Any suggestions? Thx.

-- Cronin

All,

Since Cronin asked about it, I decided to go in and re-implement the simple patch I described in my earlier post to this thread.

I added the htmlspecialchars_decode function back into the aggregator.module, removed all the old items from the news feed, then refreshed it. Viola, no more funky characters in the news feed titles.

Since several new updates/releases of Drupal have happened since my original patch, and all of them still contain the old problematic PHP code, perhaps someone can volunteer to help me get this fix added to the next version of Drupal so we don't all have to keep re-patching every time a new release comes out.

Please feel free to respond to this thread and/or email me if you can help make this happen.

Cheers,

Jason

Oops, I shouldn't have changed the patch status since it looks like the original patch isn't the one we trying to implement. I'm setting back to 'code needs work' until someone can create and submit a patch that meets the Drupal patch standards. Here is what I'm proposing someone help me do:

Write a valid patch that replaces line 737 through 741 with the following:

As soon as that patch is up and tested I'll work on getting it into the queue for the next Drupal release.

Cheers,

Jason

Jason,

"I just looked at the Google News Feed 'UCSD In the News' you mentioned on my site and noticed it does in fact have the ampersands in the links. Are you seeing something different on my site?"

No, your site looks fine. But my site (http://www.zts.com, see the section "Google Web Search Filtered for Recent Thermoelectric Content"). Check the links: "&"s are stripped out, and broken. I checked and they are stored in the DB that way too.

I just upgraded to drupal 6.9 (a day before 6.10 came out!).

Any thoughts on what's going on? Thanks for the replies.

-- Cronin

Hi everyone,

I've created and tested the attached patch on my Drupal 6.10 site and am looking for volunteers to do additional patch testing.

The patch chages the aggregator.module as generally descibed in my posts above. It first checks to see if the server has the htmlspecialcharacters_decode() function available (PHP 5.1.0 and greater). If so it uses the function to clean up HTML in the article titles and saves the results to the database. If the function isn't there for some reason, it defaults to the prior method for saving article titles.

I'll give it a few weeks for testing, then if I don't hear anything I'll assume it's good to go and try to get it into the next patch release.

Thanks,

Jason

P.S. Thanks to domesticat for turning me on to WinMerge!

I fear there may not be sufficient traffic on this thread to get additional patch testing done. After waiting a few weeks for testers to evaluate the final version of the patch, and not getting any responses, I've gone ahead and updated the status to "reviewed & tested by the community".

While nobody seems to have tested the final version, it was tested and improved by a number of users since it was first created in May of 2006 by Steve Dondley. Since those earlier iterations led us to this final version of the patch, I think it has been sufficiently tested to be committed to the next version of Drupal.

I apologize in advance for not being able to get other developers to provide a supporting opinion on the patch. However, I'm confident this patch will work just fine, particularly since the final patch checks to make sure the htmlspecialchars_decode() function is available on the server before running it.

Cheers,

Jason

I've been having problems with my aggregator in that my feeds are displaying HTML code. I've got Drupal 6.10 installed. This patch sounds like exactly what I need. I clicked on the attachment, but it appears to display some text in my browser. How do I download and install this patch?

This might work.

Thanks for re-rolling this Alex_b, sounds like we're ready to test & patch

Re-rolled for current D7 HEAD.

The last submitted patch failed testing.

Re-test of 61456_html_decode_titles_2.patch from comment #55 was requested by Arancaytar.

subscribing

LOL, I don't think this one is really that bad, and it's finally in 'needs review'. (I was trying to update the versions on the more promising issues).

I started writing a test for this for verification/assurance. The included patch adds this test but does not include the patch in #55, (nor any other proposed changes in this issue,) in order to allow testing with-and-without such changes.

The test in its current form adds a feed with an item whose title includes a quote (") and an ampersand (&).

However... these look to be displaying properly -- using SimpleTest and adding the feed manually -- without patching.
Am I testing this properly?
Is anyone actually seeing this error in D7?

I had some issues with testing the apostrophe ('). I see the correct output (manually and in SimpleTest's verbose results) when I use it in the test feed item's title, but assertText() doesn't seem to acknowledge it.

Changing this issue back from RTBC seems appropriate.

@jeffschuler

Now if you can just combine the patch and the test into one, set the issue to Needs Review and let TestBot try it out.

So you are suggesting that this actually got fixed somewhere else?

Either it's been fixed elsewhere or I'm not testing it properly.

I just tried the same test feed (generated in #67) in Drupal 6 and the entities displayed properly. So, perhaps I'm not testing the correct conditions?

The test uses a manually-generated RSS 0.91 feed (copied from the existing RSS091 test file) whose only item's title includes & and " entities.

I've added items to this feed using the title phrase "It&39;s an Honor" (from comment #1,) as well as a few other examples folks cited as problems in this issue, and they all displayed properly in D6 and D7.

Maybe someone experiencing this issue could point us to a feed they're having a problem with?

Sounds reasonable to me.

smscotten: can you point us to the feed you're having issues with?

I saw this issue recently also, but same thing as above; it turned out that the feed itself (on the source site) was incorrectly escaping its titles. So there's no bug here for Drupal to fix, at least not in D7/D8.

If an actual bug is still reproducible in Drupal 6 (i.e. with a feed that is formatted correctly on the source site but broken when imported to Drupal), someone could feel free to set this issue back to 6.x.

In the meantime, the tests in #67 look pretty solid so it's worth getting the patch rerolled and committed; tests are good regardless of whether the bug itself exists...

Patch looks good to me. The tests pass, and they're completely consistent with the way the other tests in the same area of the code are written.

I did a quick reroll to remove the "No newline at end of file" issue from the newly-added file. This should be good to go.

Haha, you heard me coming! But also....

Indentation is goofed on the second and third lines. Needs one more space before the asterisks.

Oh, and yeah.

This appears to have broken tests. Failure on "Quote" Amp&" found http://qa.drupal.org/8.x-status

Patch suggested by #87 and #89.

Note that since HEAD is broken, this is a critical bug.

Sorry about that. I just committed the patch in #91. I think that should fix HEAD. Let's see ...

Looks good - seems like we can still use a patch for D7.

Regarding assertText() vs assertRaw()... either should be fine here since it's just a text string. Leaving it as assertText() actually has the nice property that it verifies that filter_xss() doesn't do anything funny to " or ;&amp (which it shouldn't), but I suppose that's an extremely backwards way to test that :) Either one is OK.

The last submitted patch, core_aggregator_title_entities_test-61456-d7-99.patch, failed testing.

Should probably use assertRaw() rather than assertText() to be consistent with what went into Drupal 8.