system_send_email_action uses $account without initializing it

Every use of system_send_email_action will get this warning... because $account is used but never initialized. There will also typically be a warning "Trying to get property of non-object" from user_preferred_language() in user.module, line 3100.

Here is my take on what was needed here. Note that where possible it uses the recipient's language preference, falling back to the current user's language preference.

Tests for this are in #721086: Create tests for system actions, clean up token replacement, clean up triggers, which is dependent on #601398: Simpletest does not allow assigning actions to triggers.

Are you sure it's a good idea to load recipient by data provided by user input?

IMO, loading a user by email address should be a safe operation. If the email address matches, it loads. If it doesn't, we don't use it.

I'm open to correction on this.

@rfay seems that anonymous user can leave e-mail of some registered user to provide a spam sending

It's looking up a valid *user* by email. And it is conceivable that bogus users have been created, but that's a system admin problem, isn't it? Keep talking. I'm listening.

Basically:
1. You let somebody with admin privileges create an action. They're responsible for their own actions :-)
2. Nothing is being done here that is not within the realm of normal operations. An email is being sent to an email address. If that email address corresponds to a user, we use the user's preferred language.

Note that *all* that is being done here is to look up the user's preferred language. Nothing else. We're not choosing to whom to send the email or anything like that. Just looking up a user so we can get the preferred language.

@rfay I missed that only $language used from loaded user account, digging into code helps!

Improved tests are waiting for this one, so if we could get it commited, it would be cool. (Currently the assertion fixed here prevents any real (succeeding) tests of system actions.) #721086: Create tests for system actions, clean up token replacement, clean up triggers

Patch still applies, bumping this

#721086: Create tests for system actions, clean up token replacement, clean up triggers is waiting for this one too. The tests in it can't succeed until the exception in this one is fixed.

I'm not sure it makes sense to fall-back to the current user's language. If I'm one of very few people that has my language set to Dutch, but the system's default is set to English because the majority of the people are English, wouldn't it make more sense to default to English instead? It seems like we're making assumptions that might not be consistent with the rest of Drupal (but I could be wrong) -- shouldn't we loop in the language negotiation code?

I definitely agree - it should fall back to the system default. This patch does that instead.

Fall back to language_default() make sense.

@Dries, @webchick, if you could take another look at this and commit, it would be appreciated. A number of new tests and dependent on this, as we can't currently get any test to pass that uses system_send_email_action.

Seems reasonable. Committed to HEAD. Thanks.