URL-aliased links sample

The problem I think is due to the lack of a default value in two calls to variable_get().

variable_get('emailpage_sender_addy', '') should be variable_get('emailpage_sender_addy', "NoReply@".$host['host']).

Right, I made a patch. However, I've been working on a number of other issues with the module as well (including solutions by other contributors in different issues) – so the patch is pretty big. Here's a list of what I did:

• Specified a consistent value for 'emailpage_sender_addy' by assigning it at the top of the file, as is already done with 'emailpage_subject_line' etc. That fixes the problem in this issue, i think.
• Added a setting for 'emailpage_success_msg' and did the default value same as above.
• Added support for the placeholder %someone in the subject line (it is already supported in the message text).
• Changed some UI texts to reflect these changes, as well as some others where I thought there was room for improvement.
• Included an available fix for the 'double slashes' problem.
• Made the success message work (with drupal_set_message() ).
• Added a t() for 'Send it now!'.
• Changed #tree to FALSE in the emailpage_settings() form. This already available fix solves the issue with non-sticking settings.

I hope you guys are ready to review this patch, even though it deals with so many things at once. If it's committed I think we'll solve a lot of small problems with one swift stroke. I feel confident enough to use it on a live site myself...

I've tried the patch, and it does seem to make things better, but I want to make some additional changes if I'm going to be the one to apply it to CVS. The whole use of the referrer and flood cookie implementation breaks things for me if I load one page, then load another then try to email the first. I've started on an alternate implementation using $_SESSION and arg(), but it doesn't really apply to this bug report.

deekayen, I've hade similar problem as you, but I decided to ignore them until this patch was done. But I guess we're past the stage of doing things step by step (I know, I started it). Looking forward to your improvements.

However, I've found a serious bug with the current implementation. It's the code that is executed in the global scope at the top of the file. It seems harmless because there's just a couple of constants being assigned, but the calls to t() cause a lot of trouble. Those calls are executed before the $locale variable has been set, which breaks locale's caching mechanism and cuts the performance of the site drastically. It's quite bad actually.

So I moved those constant assignments to emailpage_menu() instead. (I think that hook is often used for code that a module needs to run on every page load.)

My modified version of the module is attached. (Let me know if you need a new patch.)

I handed the bit of work I've done over to AjK, but I'll still get updates to commit changes to CVS if they're posted here.

As deekayen says, I've taken over this bit of work.

Attached to this is a brand new version of emailpage.module It's sent in full as it's pretty much so different from the original a patch was really not an option.

I've tested this module on 3 different platforms and all appears well but it needs testing and feedback from the community.

This new version addresses a number of previous issues, in particular, where the user is sent after the email goes. In this version, the user is sent back to the original node and a message is added noting that the email went.

Another major point is the number of options in "admin >> settings >> emailpage". There are alot more settings (remember aswell that in control access a user needs admin perm in order to get to the admin page).

Looking forward to feedback,
best regards
AjK

Note, when saving the file above, please ensure you save it as "emailpage.module" and not "emailpage_2.module" as it's stored here as

regards
AjK

I looked through the code quickly. No time to test in now, but I will. It looks great! I like the new mechanism for default values.

A few comments:

Line 99 & 153: Should '

' be there? Other return statements in the same function don't have that. And if so, maybe we should use return '</p>' . t('string'); instead?

Line 108: The variable $local_nid should be moved so that it doesn't generate multiple instances of the locale string: t('Supplied node nid number invalid: %nid', array('%nid' => $local_nid))
And maybe it should read 'Supplied node id...' instead, nid is short for 'node id' right?

Line 153: Same thing as 108.

Line 290: Same thing again. variable_get() should be moved out of the locale string.

lanesharon's problems with path aliases are probably due to some paths in the code that aren't passed through the proper functions, such as url(). I'm not that well acquainted with those functions, so I'd have to do some research before I can help out with that.

Sorry, I messed up. It should read:

Line 99 & 153: Should </p> be there?

Thanks for the feedback,

lanesharon, I have to plead some ignorence here, can you give me a sample of a "URL-aliased links" please so I can break it down and look further.

zoo33, thx for the comments, I'll take a look at this shortly.

regards
AjK

Re-write so, have started new issue as I believe all the above has been addressed.

Please see http://drupal.org/node/63624 to follow up

regards
AjK