I've got a drupal site name that has a single quote in it, e.g. Foo's Bar. When notify sends an email, otherwise working great, the subject line is Foo's Bar new content notification for admin.

CommentFileSizeAuthor
#8 notify-5-148454-1.patch5.62 KBStandart

Comments

mygumbo’s picture

Argh. It shows up as

Foo&#039 ;s Bar

nicokarp’s picture

I had similar behaviour with single quotes within title (nodes or comments).
I did a little hack by replacing a line in notify.module :
"$body = $node_body . $comment_body;"
to
"$body = notify_entities_to_utf8($node_body . $comment_body);"

For your problem, same thing could be done for the variable $subject

I did this very quickly... working for me but not clean (newbie as i am).

martinship’s picture

Priority: Minor » Normal

Single quotes aren& #039;t the only problem: that& #039;s the apostrophe too. It& #039;s visually ugly to say the least. Since contractions are pretty common, I& #039;d say this is a normal bug, not a & #039;minor& #039; one. I& #039;m sure you see my point.

Sorry, I& #039;ve just seen this in all my notification emails and it& #039;s driving me nuts ...

gracearoha’s picture

I also have this problem with double quotes showing up like: "My Pages"

this is also driving me crazy. Has anyone found a solution to these odditites?

merilainen’s picture

I have the same problem. First I thought it has something to do with FCKeditor, but it doesn't. Email notifications for Organic Groups are pretty much unusable, as single quote is quite common character.

Standart’s picture

Title: Site name with single quote » Special chars in email are escaped using HTML entities
Version: 5.x-1.0 » 5.x-1.x-dev

Many variables like sitename, title, author, etc are run through htmlspecialchars() which encodes '&', '"', ''', '<', and '>'. This makes sense if you're displaying the variables in HTML which we don't do here. The escaping takes place because of the usage of the '@' decorator for variables in the t()-function which in turn uses check_plain() which contains htmlspecialchars(). The docs say that you should use the '!' decorator for variables supposed to appear in an email (http://api.drupal.org/api/function/t/5). Why does notify use '@'? Also a statement like

  t('@title', array('@title' => $node->title))

doesn't make much senseto me as you translate a varibale only which is the same as doing

  check_plain($node->title);

if I understand this correctly. Shouldn't we change all decorators from '@' to '!' in t() and also remove statements like the one above completely (we don't need to check for security in an HTML context at all, do we)?

matt2000’s picture

I've noticed this issue while I've been doing some work on making messages templatable in notify by views.

If someone can provide a patch for notify, I'll apply it & release, but if I have to do it myself, I may not get to it for a little while yet.

Standart’s picture

Status: Active » Needs review
StatusFileSize
new5.62 KB

Attached is my patch against 5.x-1.x-dev. I replaced all @ decorators with !. Also I removed the empty t() for the item title.

matt2000’s picture

Status: Needs review » Fixed

Initial glance at patch look good, so it's been applied to -dev branch.

Please test & report back.

matt2000’s picture

Also applied to 6.x -dev branch.

Standart’s picture

Please apply it to the 5.x-dev branch, too.

matt2000’s picture

Hmm... trying to figure out what happened to the tagging with this module.

In the meantime, a 5.x release is here: http://drupal.org/node/303336

Anonymous’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.

Standart’s picture

Status: Closed (fixed) » Reviewed & tested by the community

Please apply to the 5.x-1.x branch and make a new release 5.x-1.2.

neclimdul’s picture

Ran across this patch on a random google search.

Knowing how bad some email clients can be about running scipts(not saying names but I think we all know) this kinda scares me. People aren't as good about keeping them up to date either.

Shouldn't you run some sort of filter on some of this information at least? It will look goofy but drupal will let you put <em>test<script>alert('hello');</script></em>testing into a title. It just takes one email client flaw and...

Standart’s picture

notify.module uses PHP's strip_tags() (Version 5) or drupal_html_to_text() (Version 6) respectively so don't worry. We're not sending HTML here.

Standart’s picture

Status: Reviewed & tested by the community » Fixed

Looks like it's been committed. Thanks.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

chris55’s picture

Version: 5.x-1.x-dev » 5.x-1.3

The patch in comment #8 notify-5-148454-1.patch is flaky because the replacement has been done too thoroughly.

In particular, the format_plural() function assumes the use of @ not ! so all the counts generated by the module now show up as !count. So please change those back again. (3 instances)

e.g. t('Recent content - @count', array('@count' => format_plural(count($nodes), '1 new post', '@count new posts')))

chris55’s picture

Status: Closed (fixed) » Needs work

see above

Standart’s picture

Is this valid for the Drupal 5 version? See http://drupal.org/node/316234#comment-1041160 for Drupal 6.

gisle’s picture

Status: Needs work » Closed (won't fix)

Verified that this is not an issue is not present in 6.x-dev and 7.x-dev-branches.

It is not going to be backported to 5.x.