Support from Acquia helps fund testing for Drupal Acquia logo

Comments

BWPanda’s picture

Status: Active » Needs review
FileSize
2.41 KB

This patch seems to do the trick, but someone should check to make sure I implemented the suggested method correctly...

Dave Reid’s picture

It would be good if we could also run $settings['text'] through token_replace() using $data[$object_type] = $object.

BWPanda’s picture

Title: Allow users to edit the text 'Email contact form' used on the 'Email contact form' formatter » Allow changing the 'Email contact form' link text and form title
FileSize
4.28 KB

Good idea!
I also added the ability to change the email form's title (also using token replacement).

Some possible issues:

  • I obviously couldn't replace the tokens in the settings form summary (but I guess that's as it should be)
  • The email form has no way of knowing which view mode was used to create the link to it, so I had to get the title from the hard-coded 'default' view mode setting (however this also seems to make sense (in a way) as you don't really need to change the email form based on whether the link was in a teaser or a full-node... do you?)

Review away!

ethnovode’s picture

Thank you!
I was able to change the link using tokens. The form title didn't change but it could be because I use the Display Suite module.

broon’s picture

Status: Needs review » Reviewed & tested by the community

I applied the patch to the latest stable version (7.x-1.1) and it works like a charm.
I also have Display suite installed and couldn't find an error so far.

Just for clarification (for new users): On manage display page for a content type you are able to set link text and form title. Save the settings and it works without clearing cache. If you do not change the settings the link in content will disappear due to $settings['text'] not set.

BWPanda’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
4.4 KB

Here's an update to the patch in #3 that disables the sanitation of the Email form title (I had set the form title to 'Email [node:title]' and as my node's title was O'Meara Wood & Associates, my form title displayed as Email O'Meara Wood & Associates).

oneidprod’s picture

Thanks for your work on this as it was exactly what i was looking for. I applied the patch in #6 to the latest stable version 7.x-1.2 as well as the latest dev version 7.x-1.x-dev and the "Link Text" worked as designed, but the "Form Title" never changed regardless of any token that i tried. I also tried the patch in #3 on 7.x-1.2 and got the same results. I have display suite installed but not using for this content type, but went ahead and disabled it and it didn't matter, same results.

After having the cache clear itself, i noticed that the Email Contact Form Title goes away completely when using this patch. The <h1 id="page-title" class="title">Email Contact Form</h1> isn't even there anymore. So I don't think that it is so much that it's not changing the title as it is just being removed all together and because it was still cached I was still seeing the default title.

I'm not a coder, but I can test anything you want to try if you get time.

jlongbottom’s picture

Patch in #6 is great, but only works for default display type. I am showing an email field that is coming from a custom display type (using display suite) or a Teaser, so this fails because the module hard-codes the settings to come from the default display type. Not sure what the best solution is. I guess it could loop through each display type until it finds one with data in the 'settings' element.

We get the field info in email_contact_form_title:

$field = field_info_instance($object_type, $field_name, $bundle_name);

Here is a dump of $field:

Array
(
    [default] => Array
        (
            [label] => above
            [type] => email_default
            [settings] => Array()
            [module] => email
            [weight] => 4
        )

    [node_reference] => Array
        (
            [label] => hidden
            [type] => email_contact
            [weight] => 3
            [settings] => Array
                (
                    [text] => Contact facility
                    [title] => Contact facility
                )

            [module] => email
        )

    [teaser] => Array
        (
            [type] => hidden
            [label] => above
            [settings] => Array()
            [weight] => 0
        )

)

EDIT:

Ok, so looping through worked out ok. Too lazy to make a patch, so here is the whole, updated function:

/**
 * Generates the title for the email_contact form.
 */
function email_contact_form_title($object_type, $object_id, $field_name) {
  // Get field's bundle name
  $entity = entity_load($object_type, array($object_id));
  $bundle_name = $entity[$object_id]->type;

  // Get field instance data
  $field = field_info_instance($object_type, $field_name, $bundle_name);
  
  //get the correct display type
  //eg: default, teaser, etc.
  foreach($field['display'] as $key=>$display) {
	if(!empty($display['settings'])) {
		$display_type = $key;
	}
  }

  // Return form title
  return token_replace($field['display'][$display_type]['settings']['title'], array($object_type => $entity[$object_id]), array('sanitize' => FALSE));
}
fearlsgroove’s picture

Category: task » feature
FileSize
6.1 KB

Using the default display for settings is a bummer. Here's a modified approach:

  • Uses an instance setting for the email form title. Seems like since this is a full page view, we should facilitate linking to that independent of an instance or formatter, unless we also want to include that data in the URL itself.
  • Adds a formatter to output a link to the contact form as a plain URL. This lets us use it flexibly in views. As an example, I might include my first name field on the view, exclude it from display. Then include the email url field, rewrite, output as a link with the first name token in the text and the email as the url.
fearlsgroove’s picture

Sorry that last patch was crap. Beyond the syntax error, there's also a problem where if the formatter is not configured for the field, it returns not found when trying to just visit the URL.

Here's another attempt that adds an instance setting to allow the form to be used without a formatter being configured. Maybe this should be a permission instead? I'm not sure what the logic behind restricting the email form page to a configured field might be. Security I assume?

realityloop’s picture

Status: Needs review » Reviewed & tested by the community

works for me. Thanks.

boyan.borisov’s picture

The last patch works fine but I am uploading a new one just put the text in t() function in order to be translatable.

mh86’s picture

Status: Reviewed & tested by the community » Needs work

Thanks to all for the work on this issue. I was just taking a look at the last patch, and have a few notes:

  1. +++ b/email.module
    @@ -110,12 +198,21 @@ function email_field_formatter_view($object_type, $object, $field, $instance, $l
    +        $element[$delta] = array('#markup' => l(token_replace(t($settings['text']), array($object_type => $object), array('clear' => TRUE)), 'email/' . $object_type . '/' . $ids[0] . '/' . $instance['field_name']));
    

    t with dynamic text ($settings['text']) is not recommended. As far as I know we should rather take a look at i18n(_field) for such use cases.

    Furthermore, token_replace() sanitizes the text by default, the same as l() does, so I think we have a double escaping here

  2. +++ b/email.module
    @@ -110,12 +198,21 @@ function email_field_formatter_view($object_type, $object, $field, $instance, $l
    +    case 'email_contact_url':
    +      $ids = entity_extract_ids($object_type, $object);
    +      // As with email_contact, this only works with the first item.
    +      foreach ($items as $delta => $item) {
    +        $element[$delta] = array('#markup' => 'email/' . $object_type . '/' . $ids[0] . '/' . $instance['field_name']);
    +        break;
    +      }
    +      break;
    

    Not really sure if we need a separate formatter, you could directly do it in views on your own (url rewriting with token replacements)

  3. +++ b/email.module
    @@ -269,6 +366,21 @@ function email_mail_page_access($entity_type, $entity, $field_info) {
    +  return token_replace($field['settings']['form_title'], array($object_type => $entity[$object_id]), array('sanitize' => FALSE));
    

    We should have a fallback to the default title, if the form_title is not yet set (on updating the website, this setting will be empty).

  4. +++ b/email.module
    @@ -314,15 +426,17 @@ function email_mail_page($object_type, $object_id, $field_name) {
    +  if (!$settings['settings']['allow_without_formatter']) {
    

    Again, this setting will be empty on updating the module/website

bisonbleu’s picture

FileSize
6.93 KB

[updated]

Patch in #12 works as designed.

I had an issue in Views when the email field was using a relationship.
But it was no more than a bad configuration on my part.
So disregard the attached export.

Cheers : )

joeyenglish’s picture

Awesome solution. Question is, where is this changed? I can't seem to find the way to edit the field, as it still says "Contact person by email."

bisonbleu’s picture

@joeyenglish, after applying the patch, go to the Manage Fields tab of your content type and edit your email field (e.g. admin/structure/types/manage/[name_of_your_content_type]/fields/name_of_your_email_field). In the field settings, look for the form field Email form title.

redndahead’s picture

Status: Needs work » Needs review
FileSize
7.56 KB

This version incorporates items 1a and 4 from #13. It also adds a special case for taxonomies that have a contact field. That caused the error:

Notice: Undefined property: stdClass::$type in email_contact_form_title() (line 384

realityloop’s picture

Status: Needs review » Needs work
Issue tags: +Amsterdam2014
+++ b/email.module
@@ -99,10 +140,57 @@ function email_field_formatter_info() {
+      '#description' => t("Change the wording of the link text. Can use token replacement. Defaults to 'Contact person by email'."),

Just wondering if there is there any reason you use double quotes here instead of single with escaping like you have with the other instances of t()? I think it may be better to change that, otherwise this looks good to me.

MiroslavBanov’s picture

The version in #17 removes the t() and doesn't add any i18n handling either.
Here is a patch with the t() back, and also with tokens in popup (Node tokens tree kill my browser).

umakart’s picture

#19 works for me. Thanks a lot. Why not committing?

MiroslavBanov’s picture

Status: Needs work » Needs review
Issue tags: -Amsterdam2014

Needs review before committing. Also note that #18 is not addressed.

fearlsgroove’s picture

@MiroslavBanov -- you can't use t() on user provided values, only hard coded values. See docs for t() for details. I don't know what the right way to handle i18n is here as i'm not translation expert, but that bit shouldn't be committed.

MiroslavBanov’s picture

Status: Needs review » Needs work

Right. Then I guess it needs work.

capfive’s picture

FileSize
21.38 KB

couldn't apply this so i applied it manually... to 7.x-1.3 here is the .txt file :)

p.s it is working very well!

capfive’s picture

***UPDATE!!!!*** Not playing nice with paragraphs bundle

i added the field to a paragraph bundle for a team display and receive the following error pops up on the form page, but the form does send.

Notice: Undefined property: ParagraphsItemEntity::$type in email_contact_form_title() (line 383 of /public_html/sites/all/modules/email/email.module).

here is the line that i put in.

/**
 * Generates the title for the email_contact form.
 */
<?php
function email_contact_form_title($object_type, $object_id, $field_name) {
    // Get field's bundle name
    $entity = entity_load($object_type, array($object_id));

    // Special casing for taxonomy fields.
    if ($object_type == 'taxonomy_term') {
        $bundle_name = $entity[$object_id]->vocabulary_machine_name;
    } else {
        $bundle_name = $entity[$object_id]->type;
    }

    // Get field instance data
    $field = field_info_instance($object_type, $field_name, $bundle_name);

    // Return form title
    return token_replace($field['settings']['form_title'], array($object_type => $entity[$object_id]), array('sanitize' => FALSE));
?>
}

with $bundle_name = $entity[$object_id]->type; being on line 383.

Thanks!

MiroslavBanov’s picture

@capfive
Should be replaced with something like

  list(, , $bundle) = entity_extract_ids($entity, $entity_type);
capfive’s picture

@MiroslavBanov

got some other issues when replacing that code

Notice: Undefined variable: entity_type in email_contact_form_title() (line 387 of /home/totalconstruction/public_html/sites/all/modules/email/email.module).
Warning: Illegal offset type in isset or empty in entity_get_info() (line 7727 of /home/totalconstruction/public_html/includes/common.inc).
Notice: Undefined variable: bundle_name in email_contact_form_title() (line 391 of /home/totalconstruction/public_html/sites/all/modules/email/email.module).
Notice: Undefined variable: entity_type in email_contact_form_title() (line 387 of /home/totalconstruction/public_html/sites/all/modules/email/email.module).
Warning: Illegal offset type in isset or empty in entity_get_info() (line 7727 of /home/totalconstruction/public_html/includes/common.inc).
Notice: Undefined variable: bundle_name in email_contact_form_title() (line 391 of /home/totalconstruction/public_html/sites/all/modules/email/email.module).

happy to keep testing for you if you would like to have another go at it :)

MiroslavBanov’s picture

@capfive
OK, here's the corrected code

  list(, , $bundle_name) = entity_extract_ids($entity[$object_id], $object_type);
MiroslavBanov’s picture

Now I finally had to fix this for field collection item - here is the patch. #22 is still not fixed though.