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>

CommentFileSizeAuthor
#4 block.module.patch_2.txt 482 bytesoadaeh
#1 aggregator-striptags.patch 522 bytesahoeben

Comments

ahoeben’s picture

Comment #1

ahoeben commented
Version: 4.6.3 » x.y.z
StatusFileSize
new aggregator-striptags.patch 522 bytes

Attached patch is against CVS, and strips tags before inserting in the database

drumm’s picture

Comment #2

drumm
he/him
Location NY, US
commented
Status: Needs review » Needs work

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.

ahoeben’s picture

Comment #3

ahoeben commented

There'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.

oadaeh’s picture

Comment #4

oadaeh commented
Status: Needs work » Needs review
StatusFileSize
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. :^)

webchick’s picture

Comment #5

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Needs review » Needs work

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'.

webchick’s picture

Comment #6

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

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.

oadaeh’s picture

Comment #7

oadaeh commented

In 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.

webchick’s picture

Comment #8

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

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.

narres’s picture

Comment #9

narres commented

I 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 ;)

oadaeh’s picture

Comment #10

oadaeh commented
Version: x.y.z » 5.x-dev

I'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.

bdragon’s picture

Comment #11

bdragon commented
Status: Needs work » Closed (fixed)

Queue maintenance brought to you by patch bingo.