I think the line should be:
$output .= ' <div class="title"><a href="/'. check_url($item->link) .'">'. check_plain(strip_tags($item->title)) ."</a></div>\n";
, cause some feeds are containing formating in xml-tag .
This makes a stupid <a href="http://narres.it">Narres <b>IT</b></a>
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | block.module.patch_2.txt | 482 bytes | oadaeh |
| #1 | aggregator-striptags.patch | 522 bytes | ahoeben |
Comments
In most cases we avoid filtering before saving in the database so the original is maintained and can be seen in editing. This doesn't have editing at the moment, but I think we should follow the same convention.
Narres's solution looks okay on initial review, but needs a patch file and testing.
Comment #3
ahoeben commentedThere's a rationale behind not filtering before inserting into the database; this is done for consistency, so a user who edits a previous post gets his exact input back. That rationale does not apply here; there's no user editing a post.
Furthermore, in _comment_form_submit, the title is also filtered if it is generated from the body.
Comment #4
oadaeh commented| Status | File | Size |
|---|---|---|
| new | block.module.patch_2.txt | 482 bytes |
I'm going to follow up here, because I think it is the closest thing the the bug I found. Correct me if I am wrong.
narres, are you referring to the titles that are displayed when configuring a block, like this?:
'<em>Drupal</em> category latest items' block
I'm getting those sorts of titles when I'm configuring aggregator blocks.
If so, then the fix is in the block module and not in the aggregator module. Attached is my patch, where I applied narres' idea.
ahoeben, your patch would actually change the way the aggregator categories and feeds are displayed everywhere through out the site, and the problem is only in block configuration titles (that I've been able to see).
Now, if I'm totally off base on this, I'll go create a new bug against the block module. :^)
Ah. Nope, the fix for this is actually using one of the new placeholders for t(), rather than %, which does theme_placeholder. I think probably @, which does a 'check_plain'.
and I think this will be a problem in the calling module (aggregator) rather than in block module which looks to be doing what it should.
Comment #7
oadaeh commentedIn my patch, I don't actually use check_plain(), but strip_tags(). I could change %name to @name, but I still need the strip_tags function to get rid of the stray em tags.
Also, there is no code in aggregator.module for setting the title of the block configuration page, that I was able to determine. It's all done in block.module. All that's being done in aggregator is adding a select field for the number of items to show.
Right.. I'm saying the reason it's outputting:
<em>something</em> blah
is because whatever's passing in at message has its placeholders as %something, rather than @something, so it's getting theme_placeholder()ed twice.
Comment #9
narres commentedI did it as in above example. Otherwise I would get results like at: http://pressemitteilung.ws/aggregator/sources/5 which shows wrong link texts. It's hard to post here cause the same wrong handling is here on this site, but easy to show.
Cause we are meeting next week in Brussels, I will show it ;)
Comment #10
oadaeh commentedI'm revisiting this, cause it's in my issues queue, and I cannot reproduce it in 5.1.
Is this still a valid bug? If so, please tag it with the correct version (it was tagged x.y.z from the version revision some time ago). If not, please close it. If there is no response in a week or so, I'll tag it as fixed or closed or something.
Comment #11
bdragon commentedQueue maintenance brought to you by patch bingo.
Comment #1
ahoeben commentedAttached patch is against CVS, and strips tags before inserting in the database