I have a patch for an earlier (4.6) version of this module that allows you to forward any url, not just nodes. I'd be happy to offer it here, though it would mean some significant changes now since you've add the click-tracking stuff, for example. Is there any reason that such a feature shouldn't be incorporated into this project (i.e., tell me so I don't waste my time ...).

CommentFileSizeAuthor
#21 forward_5.patch13.56 KBadixon
#14 forward_4.patch13.57 KBadixon
#6 forward.install.patch563 bytesadixon
#5 forward.theme_.patch4.55 KBadixon
#4 forward.module_3.patch14.17 KBadixon

Comments

moshe weitzman’s picture

moshe weitzman commented

seems like a reasonable idea to me ... note that i am not ndoing any long term maintanience of this module. i updated it a bit for a client recently. Still needs more updating/cleaning.

seanr’s picture

seanr
he/him
commented

If you post the patch, I'll take a look at it.

adixon’s picture

adixon commented
Assigned: Unassigned » adixon

okay, thanks for your answers, patch coming soon.

adixon’s picture

adixon commented
Version: 5.x-1.x-dev » 7.x-1.x-dev
Status: Active » Needs review
StatusFileSize
new forward.module_3.patch14.17 KB

Okay, here are three patch files for forward.module, forward.theme and forward.install.

The goal of this patch is extend the functionality of the /forward callback url to forwarding non-node pages. Some of this was already there from what looks like the postcard feature, but this patch now supports three different kinds of forwards:

1. postcard, no change (looks like legacy support only)
2. node pages, the previous functionality
3. non-node pages, the new functionality

The essence of the patch is to change the forward url from:

/forward/

to

/forward/node/

for node pages, and in the process to allow arbitrary

/forward/

formats.

The way this would be useful is to add an 'email this page' link at the theming level with a little php, as demonstrated (for example, this is actually in drupal 4.6) here:
http://share.ca/en/shareholderdb

To use the patch, just apply normally, and then run the update script. The update script just updates the variables table so that the names are more consistent.

The patch could have been smaller if I created a new callback for these non-page nodes, but i'd say the module already suffers from bloat due to backward compatibility so I'm aiming this as one step of a bigger cleanup. I thought i'd better submit them in pieces to avoid gagging ...

adixon’s picture

adixon commented
StatusFileSize
new forward.theme_.patch4.55 KB

here's the theme patch

adixon’s picture

adixon commented
StatusFileSize
new forward.install.patch563 bytes

here's the install patch

seanr’s picture

seanr
he/him
commented

Anyone able to help test this on some 5.0 sites? Nearly all of mine are live sites right now so I could use some extra help. BTW, patch should apply equally well on both head and 5.0 branches. AFAIK there are no differences between the two branches yet (I haven't done any 6.x development yet).

phillipadsmith’s picture

phillipadsmith commented

I've asked a colleague to test it on a site that we maintain that was recently upgraded to 5.0. He should post the results shortly.

Phillip.

phillipadsmith’s picture

phillipadsmith commented

Sorry for the slow response here. I decided to give this module / patch a go myself and found that I'm not able to forward pages at all using this module. The link shows up on existing nodes just fine, but when I click the forward link, I don't arrive on the forward form page ... just at the /node page instead. I've checked the permissions, etc. and no luck.

Any thoughts?

Phillip.

phillipadsmith’s picture

phillipadsmith commented

I moved the from the /sites/all/modules/ directory to the /modules directory and now the basic forward functionality is working fine. Going to re-try the patch now.

Phillip.

phillipadsmith’s picture

phillipadsmith commented

The first patch didn't seem to apply cleanly for me:

--------------------------
|--- forward.module.dist 2007-04-26 12:56:57.000000000 -0400
|+++ forward.module 2007-04-26 12:56:57.000000000 -0400
--------------------------
Patching file forward.module using Plan A...
Hunk #1 failed at 61.
Hunk #2 succeeded at 69.
Hunk #3 succeeded at 176 with fuzz 2.
Hunk #4 failed at 262.
Hunk #5 succeeded at 348 with fuzz 2.
Hunk #6 failed at 358.
Hunk #7 succeeded at 377 with fuzz 2.
Hunk #8 failed at 456.
Hunk #9 failed at 508.
Hunk #10 failed at 579.
Hunk #11 failed at 618.
Hunk #12 succeeded at 646.
Hunk #13 succeeded at 684.
Hunk #14 succeeded at 832 with fuzz 2.
7 out of 14 hunks failed--saving rejects to forward.module.rej
done

I'm going to re-try these on a vanilla install and see what happens.

phillipadsmith’s picture

phillipadsmith commented

Tried again on a vanilla install of 5.1 and forward-5.x-1.x-dev:

patch < forward.module_3.patch.txt
patching file forward.module
Hunk #1 FAILED at 61.
Hunk #3 succeeded at 176 with fuzz 2.
Hunk #4 FAILED at 262.
Hunk #5 succeeded at 348 with fuzz 2.
Hunk #6 FAILED at 358.
Hunk #7 succeeded at 377 with fuzz 2.
Hunk #8 FAILED at 456.
Hunk #9 FAILED at 508.
Hunk #10 FAILED at 579.
Hunk #11 FAILED at 618.
Hunk #14 FAILED at 832.
8 out of 14 hunks FAILED -- saving rejects to file forward.module.rej

seanr’s picture

seanr
he/him
commented

The patch will have to be updated as there have been some other fairly important changes made to the module since then. Sorry for the inconvenience.

adixon’s picture

adixon commented
StatusFileSize
new forward_4.patch13.57 KB

Okay, here's a completely rewritten version of this patch, you can disregard the old one.

1. I gave up trying to modify the existing mechanism, the new security issues pointed out that forwarding a url or path is fundamentally different from forwarding a node, and trying to overload the existing mechanism was too ugly.

2. Instead the patch provides alternative ways of invoking the basic forward_form. Note that the forward_form already supports the old 'epostcard' forwarding, this patch just adds two new ways: path and referrer.

3. Invoking /forward/path/
will allow you to forward a url from the drupal site with a path of
(either internal or alias). Since we don't know much about
, we don't try to include any content (like the default node version does). There's not even any testing that
is valid.

4. invoking /forward/referrer will attempt to allow you forward a url from another domain. This is the functionality that Phillip is actually interested in, and the only extra work is to provide a way of limiting which domains can do this.

Conclusion: this patch leaves all the existing code untouched (except a minor reorganization of the admin screen). To use the new features, just provide appropriate permissions in the usual admin location, and better messages in the forward admin screen. Then you can either:

a. provide a link like l('tell a friend','/forward/path/'.$_GET['q']) in your theme (e.g. in page.tpl.php). This was the original spec for this patch.

or

b. provide a link like tell a friend from another website (whose domain would need to be put into the allowed referrers list).

I don't think the recent security issue affects the new functionality, since it doesn't actually send any content from the path/referrer.

Neither of the two new forwards are tracked. I notice that the new security patch actually breaks forward tracking of epostcards, but I'm not sure why you'd want to track these anyway. If tracking is desired, i'd recommend that we add a different table for tracking these non-node urls.

And finally - this time I figured out how to patch forwad.module and forward.theme in a single patch file! No patch to the .install file.

phillipadsmith’s picture

phillipadsmith commented

So far so good against 5.x-1.2:

bash-2.05b$ patch < forward_4.patch
Hmm... Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|Index: forward.module
|===================================================================
|RCS file: /cvs/drupal-contrib/contributions/modules/forward/forward.module,v
|retrieving revision 1.50
|diff -u -r1.50 forward.module
|--- forward.module 13 Jul 2007 19:45:25 -0000 1.50
|+++ forward.module 17 Jul 2007 15:33:34 -0000
--------------------------
Patching file forward.module using Plan A...
Hunk #1 succeeded at 21.
Hunk #2 succeeded at 39.
Hunk #3 succeeded at 245.
Hunk #4 succeeded at 310.
Hunk #5 succeeded at 338.
Hunk #6 succeeded at 437.
Hunk #7 succeeded at 572.
Hunk #8 succeeded at 604.
Hunk #9 succeeded at 618.
Hunk #10 succeeded at 638.
Hunk #11 succeeded at 662.
Hunk #12 succeeded at 846.
Hmm... The next patch looks like a unified diff to me...
The text leading up to this was:
--------------------------
|Index: forward.theme
|===================================================================
|RCS file: /cvs/drupal-contrib/contributions/modules/forward/forward.theme,v
|retrieving revision 1.5
|diff -u -r1.5 forward.theme
|--- forward.theme 17 Jan 2007 22:14:12 -0000 1.5
|+++ forward.theme 17 Jul 2007 15:33:34 -0000
--------------------------
Patching file forward.theme using Plan A...
Hunk #1 succeeded at 69.
Hunk #2 succeeded at 102.
Hunk #3 succeeded at 114.
done

Next up: does it work!? Back to you shortly.

Phillip.

phillipadsmith’s picture

phillipadsmith commented

Basic functionality for referrer is working a-ok, with the following exceptions:

If I put content in the "Forward Instructions:" input, it only appears intermittently on the forward form (most often it just doesn't appear at all).

Also, the !site is grabbing the full referrering URL, and not recognizing the text in "Forward Referrer Message Subject:" -- so I get the full URL in the message subject and message body, vs. just "!name has sent you a message from My Site."

Also seems to be doing this in the hidden inputs as well:



I'll test the other non-referrer modes now.

Phillip.

phillipadsmith’s picture

phillipadsmith commented

Actually, as far as I can tell, anything entered on /admin/settings/forward/ to customize the message subject, body, etc. is all ignored. I didn't try changing these prior to applying the patch, so I'm not sure if it was the patch that caused this issue, or if it was like that before? I can give the default install a try to see if anything changes.

Phillip.

phillipadsmith’s picture

phillipadsmith commented

Yep: appears that the patch circumvents any changes that I make to the options on the /admin/settings/foward/ screen.

However, no matter what I do, I can't get the forward instructions to show up -- any thoughts there?

Phillip.

adixon’s picture

adixon commented

hey phillip:

i suspect this is just a documentation issue.

1. The subject and message are separately stored for the four different ways of invoking the function. In other words, you probably need to change the default subject and message for forwarding urls, and/or forwarding referrers, which are below the normal node invocation versions (and by default collapsed, called "forward path form defaults" and "forward referrer form defaults").

2. Invoking as a referrer means the code doesn't know what the referring site is called, so the best it can do for !site is the url. So the default isn't very beautiful. But if you're only using the referrer function from a single domain, you can just hard code it and not use the !site replacement variable. We could fix this by changing the list of allowed referrers to an associative array with domains as the keys and names as the values (though the admin interface is already getting pretty complex).

3. For the forwarding path invocation, I decided that using the default site name (Drupal) was even worse that the default url, so the not very nice default behaviour has been changed to something that I think is a little bit less ugly. In other words, give your drupal site a name if you're going to test this module.

phillipadsmith’s picture

phillipadsmith commented

1. The subject and message are separately stored for the four different ways of invoking the function. In other words, you probably need to change the default subject and message for forwarding urls, and/or forwarding referrers, which are below the normal node invocation versions (and by default collapsed, called "forward path form defaults" and "forward referrer form defaults").

Yep: got those and hard-coded my subject line in as "!name has sent you a message from Domain Name," but I still get !site shown everywhere. So, somewhere, the subject I'm putting in the referrer subject input on the admin screen is getting overridden.

So the default isn't very beautiful. But if you're only using the referrer function from a single domain, you can just hard code it and not use the !site replacement variable.

Yep, that's what I tried.

3. For the forwarding path invocation, I decided that using the default site name (Drupal) was even worse that the default url, so the not very nice default behaviour has been changed to something that I think is a little bit less ugly. In other words, give your drupal site a name if you're going to test this module.

Forward path seemed to work a-ok, just the referrer that is acting goofy. Also, the page help / intro text isn't written to the screen with the patch installed. Perhaps there needs to be a help / intro input for each mode?

Phillip.

adixon’s picture

adixon commented
Version: 7.x-1.x-dev » 5.x-1.x-dev
StatusFileSize
new forward_5.patch13.56 KB

okay, here it is again to fix that bug (a small typo). I see that there's already been some work since this patch was submitted (for drupal 6), so it no longer merges cleanly with CVS head (though it should work for the latest 5.x release). I don't really want to rewrite this patch again, so i wonder if this couldn't be either incorporated or if i should just write a different module.

seanr’s picture

seanr
he/him
commented
Assigned: adixon » seanr
Status: Needs review » Needs work

I'm not particularly digging the way those code is written - seems like it adds a lot of fuzz rather than working with the existing code as much as possible. One particular thing that bugs me is all the referrer stuff. What specific reason would someone have to use forward to send a page from a different domain that they couldn't just install forward (or any other solution) directly on that domain? Your path also appears to destroy the tracking functionality (eliminating the forward/emailref path so clickthroughs can't be tracked).

It also doesn't appear that the patch adds an actual forward link to non-node pages, which I thought was the entire purpose? Were you intending the link to be added via a block (IIRC, there was interest in that elsewhere)?

I'm willing to work on additional features to add a link to the non-node pages (via a block or otherwise), but I'm not fond of the idea of forwarding external links - that seems quite outside the scope of what this module was intended to do. I'll plan to work on this over the next week or so.

JacobSingh’s picture

JacobSingh commented

This is really important to a current project of mine. Is there anything I can do $$ wise or brain wise to facilitate this happening soon?

seanr’s picture

seanr
he/him
commented

I'm mainly in need of some PHP help on this. I could probably figure it out given enough time, but am pretty busy right now (was in Chicago for YearlyKos, at my sister's now, and won't be back in DC until next Wednesday).

adixon’s picture

adixon commented

hey seanr:

i'm not quite sure i understand your questions about my patch. I don't think i affected any of the other modes of invoking the forward functionality. The fact that it doesn't add a forward link - that's as per the design, it's intended to be added by the theme. The point is that this new way of invoking forwarding should be available on all pages, not just node pages (i.e., there's no way to automatically generate a link on all pages).

all in all though, i agree that it's getting to be a dog's breakfast, and perhaps the distinctions are enough that I should put this in a separate module. In that case, your forward module would cover the needs of 'node forwarding' and 'e-postcard forwarding', and the new module would cover the needs of 'page forwarding' and 'referrer forwarding'.

If that sounds better for you, tell me and I'll go ahead.

wmostrey’s picture

wmostrey commented

Hi seanr,

If you're in need of PHP help to tackle this issue, feel free to contact me. I work with Jacob on this and we require this module's functionality for non-node pages as well.

phillipadsmith’s picture

phillipadsmith commented

If that sounds better for you, tell me and I'll go ahead.

+1

Sounds good to me, as -- for the project I'm working on -- I'm using the latter (referrer forwarding) to achieve the former (node forwarding).

One particular thing that bugs me is all the referrer stuff. What specific reason would someone have to use forward to send a page from a different domain that they couldn't just install forward (or any other solution) directly on that domain?

I can assure you that -- specifically for large sites that use many different tools -- this is a commonly occurring need. The idea is to keep code manageable and simple by re-using it where possible, i.e., why set-up some other PHP script to forward pages (which you then have to manage, patch, upgrade, etc.), if that same functionality already exists in Drupal. This way there just one place to maintain that code.

Anyway: idea of a separate module sounds good to me.

Thanks!

Phillip.

cyberwolf’s picture

cyberwolf commented

Any news on this one? Sounds like a very good idea to me.

adixon’s picture

adixon commented
Status: Needs work » Closed (fixed)

Forwarding of non-node pages was incorporated into the forward module some time ago, so this thread is obsolete.