The forward module's logic within forward_link() is to add an 'email this page' link on any page that is not the main page (or on the main page if that option is on). There is a fundamental assumption that any page on the site will be a node, and that every page beyond the front page will have a valid node ID.

On sites that build pages in code and/or use modules that do, this assumption breaks. The event module is a good example. With both forward and event installed, the links on /event that allow the user to pick table / month / day / week / list view are extended to include 'email this page'. The page that is sent when this link is clicked is in 'postcard' mode and is always the front page of the site, not the page that the user believed that he or she was forwarding.

Attached is a patch that checks to make sure it is dealing with a /node/nnn page or the main page before adding the email this page link.

For what it is worth, a more involved solution (not in this patch) would be to capture the URL of the page to forward, tuck it into the form as a #hidden element, then pass it into the theme engine as a $vars[] so that the theme could include a link back to the actual page being forwarded rather than to the front page, which appears to be all it does today.

CommentFileSizeAuthor
forwardmodule_link_logic.patch1.12 KBscott.mclewin

Comments

marcoBauli’s picture

marcoBauli commented

scott, yes is true what you say, actually is not possible to forward pages that are not nodes, and that's a pity because this means is not possible to forward pages generated by a lot of contributed modules.

But instead of not showing the forward link on these pages, wouldn't be better to fix this "bug"?
ok, some rethoric going on here... unfortunately i am no coder, but i volunteer in case someone needs a tester for this ;)

(also can anyone tell why i can *subscribe* to forum posts but not to issues? :o

cheers

scott.mclewin’s picture

scott.mclewin commented

I agree with you.

I looked for a way to do this as I wanted my users to be able to forward the event calendars. There is no generalized way that I know of (I suspect that there is one, but I don't know core enough to recognize it) to capture the contents section of the page after it has been generated by a contributed module to then include it in a forwarded mail message.

Without participation of each contributed module through something like a "hook_forward()" this behavior won't change.

The patch as it stands at least removes the confusing behavior that users see today where non-functional forward links are placed on module-contributed pages.

marcoBauli’s picture

marcoBauli commented

hm i see.. can i ask if would such a hook be complex to cook, and the same for any contrib module?

thx

scott.mclewin’s picture

scott.mclewin commented

Such a hook would be non-trivial. Many displays, like the event calendar, rely on CSS definitions to make the page layouts work. With HTML enabled mail the CSS needs to be directly written out within the message (there is no including it like you do on a web page) since mail needs to be readable off-line or on a network without access to the 'net and thus the CSS files. The proposed hook would require substantial duplication of code for that page, including CSS definitions and content.

There is also the little problem that more complex displays will be very badly rendered when viewed in a text only browser. Again the event calendar is the example I worked through because I really want my users to be able to forward it.

I decided against all that.

I wonder were the module maintainer is lurking...will he/she adopt my simple patch?

geodaniel’s picture

geodaniel commented

I've tried out the patch and it does as expected - I'd really like to see this get committed to the module as well as it doesn't make sense adding the link to places like the links above the event calendar.

dtj’s picture

dtj commented

I wonder whether you might have even more granularity of control over displaying the forward link by:

  1. Applying this patch.
  2. Installing the service links module.
  3. Modifying the service_links.module to include the email forward link to the service_links_render() function:

      if (variable_get('service_links_show_forward', 0)) {
    $links[] = theme('service_links_build_link', t('email'), "/forward/$node->nid", t('Forward this page to a friend'), 'email.png');
  }
  4. ...And then adding something like the following to service_links_settings():
      $form['what_links_to_show']['service_links_show_forward'] = array(
    '#type' => 'checkbox',
    '#title' => t('Show email forward link -- Note: requires forward.module'),
    '#return_value' => 1,
    '#default_value' => variable_get('service_links_show_forward', 1),
  );

The Service Links Module allows you to specify what node types to display the links on. It also gives you the ability to display the links where you'd like to see them (i.e. within the node itself or within links) and the option of text or image links.

pr1001’s picture

pr1001 commented

I did the following change. I don't necessarily recommend it, but it's a quick fix that met my needs.

/*
    if (!$main || variable_get('forward_show_on_main', 0)) {
*/
   $allowed_types = array('flexinode-1', 'flexinode-5', 'blog', 'forum');

    if ((!$main || variable_get('forward_show_on_main', 0)) && in_array($node->type, $allowed_types)) {
scott.mclewin’s picture

scott.mclewin commented

Do you require any further information from me on this patch? I'm still interested in having it committed.

seanr’s picture

seanr
he/him
commented
Status: Needs review » Fixed

The original patch has been committed to head, 5.x, and 4.7. If anyone comes up with a good way of making it possible to send non-node pages, please submit it in the issue about that and I'll look at it: http://drupal.org/node/137137

scott.mclewin’s picture

scott.mclewin commented

Thanks Sean. I appreciate having the patch committed.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)