Add the image style tokens provided by https://www.drupal.org/project/imagecache_token in 7.x

CommentFileSizeAuthor
#45 interdiff-42-45.txt4.8 KBBambell
#45 add_image_style_tokens-2611866-45.patch15.22 KBBambell
#42 interdiff-40-42.txt8.34 KBBambell
#42 add_image_style_tokens-2611866-42.patch18.33 KBBambell
#40 interdiff-37-40.txt2.75 KBBambell
#40 add_image_style_tokens-2611866-40.patch13.79 KBBambell
#37 interdiff-35-37.txt7.9 KBBambell
#37 add_image_style_tokens-2611866-37.patch13.79 KBBambell
#35 add_image_style_tokens-2611866-35.patch15.16 KBBambell
#32 interdiff-31-32.txt1.51 KBBambell
#32 integrate_imagecache-2611866-32.patch12.59 KBBambell
#31 interdiff-29-31.txt893 bytesBambell
#31 integrate_imagecache-2611866-31.patch12.1 KBBambell
#29 interdiff-27-29.txt9.67 KBBambell
#29 integrate_imagecache-2611866-29.patch11.98 KBBambell
#27 token-imagecache-token-image-with-image-style.png58.47 KBBambell
#27 interdiff-25-27.txt13.18 KBBambell
#27 integrate_imagecache-2611866-27.patch10.81 KBBambell
#25 integrate_imagecache-2611866-25.patch11.78 KBBambell
#19 d7-imagecache-token.png38.9 KBBambell
#17 interdiff-9-17.txt10.27 KBBambell
#17 integrate_imagecache-2611866-17.patch10.77 KBBambell
#13 interdiff-13-9.txt10.24 KBBambell
#13 integrate_imagecache-2611866-13.patch8.83 KBBambell
#9 port_imagecache_token-2611866-9.patch3.72 KBBambell
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

DamienMcKenna created an issue. See original summary.

Berdir’s picture

For the record, I'd be open to discuss supporting this directly in the token module which I co-maintain in 8.x.

DamienMcKenna’s picture

@Berdir: That'd be *amazing*! +1 for that idea!

silkogelman’s picture

@Berdir: awesome!

From the (content) marketing perspective it is kind of a big deal to be able to serve the correct OG:image to the og:image tag (using Metatag module + the right Token) with Drupal 8.

I'd be happy to test patches and write a step-by-step tutorial for this (those that come with neat screenshots and arrows pointing to things :).

delalis’s picture

this is needed, badly. facebook only allows a maximum of 8MB file size for sharing, so if the user uploaded a massive image file, we need to be able to specify a scaled down image style inside this og:image tag.

seancasey’s picture

+1, this would be awesome to see! To echo the comments in #4, it's fairly critical to be able to serve image styles that are tailored to the optimal dimensions for respective social media outlets.

delalis’s picture

I actually made a feeble attempt at porting the module over to D8 but got caught up on the fact that D8 no longer has certain built in function calls such as "image_styles()" to provide a list of the current image styles set up on the site for example. There are other examples as well, the documentation for this in D8 is weak and I don't have time to dig through the code/D8 API to find the equivalent for these utility functions.

Please someone (@Berdir) add this to the Token module! Would be really appreciated and help us social media junkies out a lot!

Berdir’s picture

I would recommend you upload what you have as a patch, github repository or anything like that, so others can contribute.

Given that we want it to be in token.module, it needs to be a patch against that module in the end.

image styles are now a config entity, when updating something, always check the change recoreds: https://www.drupal.org/list-changes/drupal/published?keywords_descriptio...

The one it finds there does not contain this function directly, but it points to the issues that changed the code, In this case, all that is not very helpful. However, image_style_options() still exists, you can either use that or look int there how to load the image styles, if you need more than the id and label.

Bambell’s picture

Work in progress patch against Token module. It works only for the image field right now.

Berdir’s picture

  1. +++ b/token.tokens.inc
    @@ -433,6 +434,44 @@ function token_tokens($type, array $tokens, array $data = array(), array $option
    +        if ($style->createDerivative($original_uri, $derivative_uri)) {
    +          /** @var \Drupal\Core\Image\Image $image */
    +          $image = Drupal::service('image.factory')->get($derivative_uri);
    +
    +          // Generate the replacement token.
    +          switch ($parts[2]) {
    

    I guess this was the same in 7.x, never looked at the code, I assume it's the same there.

    But creating the file and then opening it seems like a pretty slow way to get this information.

    Have a lookt at template_preprocess_image_style gets the width/height.

    mime type and file size is different, those we really only can get like this, but maybe we could add a warning that those are slow?

  2. +++ b/token.tokens.inc
    @@ -1230,6 +1269,48 @@ function field_token_info_alter(&$info) {
    +  // Provide image_field tokens for the different image styles.
    +  $info['types']['field_image'] = [
    +    'name' => t('Image field'),
    +    'needs-data' => 'field_image',
    +    'module' => 'token',
    +  ];
    

    good enough as a proof of concept, eventually we will need to make this part of the ongoing field token patches and generate it dynamically. but works for now :)

Bambell’s picture

Title: Port Imagecache Token to Drupal 8 » Integrate Imagecache Token (D7) module
Project: Imagecache Token » Token
Version: 7.x-1.x-dev » 8.x-1.x-dev
Component: Miscellaneous » Code
Category: Task » Feature request

Moving issue to token module.

delalis’s picture

awesome guys! Props to @Bambell! Glad to see you are working on this. I don't have time to test the patch right now but I will grab it as soon as you have it integrated into a dev release for the token module. Please comment here when it is in a dev release so I know it's ready :)

Thanks again!!

Bambell’s picture

Status: Active » Needs review
FileSize
8.83 KB
10.24 KB

Here we go, that should be all there is to it. It supports all fields of type 'image' within a node. I think the original module would also support 'file' fields, but I'm not sure if that should be ported too?

Status: Needs review » Needs work

The last submitted patch, 13: integrate_imagecache-2611866-13.patch, failed testing.

The last submitted patch, 13: integrate_imagecache-2611866-13.patch, failed testing.

silkogelman’s picture

needs work but tested #13 nonetheless:

Drupal versions tested:

  • 8.1.4-dev (simplytest.me)
  • 8.1.3 (local dev)

module versions:

  • token 8.x-1.x-dev (2016-Jun-22)
  • metatag 8.x-1.0-beta9

- patch applied cleanly
- token browser works: the image styles show up as tokens
- visiting the node after creating a node that has a metatag with the new tokens a 'The website encountered an unexpected error. Please try again later.' WSOD shows up. Not sure yet if that's because of token 8.x-1.x-dev or the #13 patch (testing that as we speak)

Update:
from how it looks currently the WSOD error only occurs after applying the #13 patch.
Extra info: Installing Drupal with Dutch as language, haven't tested default English yet.

Happy to do more testing for future patches of this issue.

Bambell’s picture

Here we go. This should pass tests. The main culprit was $node->get($parts[0]) on a non-field. Thanks for testing @silkogelman and @delalis.

silkogelman’s picture

Did a bit of testing of #17 during my lunch break: can't get the Token to populate the og:image tag yet.
Will test some more at the end of the day.

  1. applied patch enabled modules
  2. created the image style 'open graph'
  3. created content type 'content' with image field 'field_social_media_image'
  4. added token [node:field_social_media_image:open_graph] to metatag og (tried adding the Token in og:image under 'content' (the one that Inherits meta tags from: Global) as wel as adding a standard metatag for the content type 'content' (I admit the name of the content type I choose is not helping making this clearer :)
  5. added content with an awesome image
  6. cleared caches
  7. checked source to see if og:image is populated (it's not)
  8. repeated steps 5,6,7 + deleting/changing the image

Drupal versions tested:

  • 8.1.4-dev (simplytest.me)
  • 8.1.3 (local dev)

module versions:

  • token 8.x-1.x-dev (2016-Jun-22)
  • metatag 8.x-1.0-beta9

And to be complete:

  • The WSOD error that #13 produced is gone
  • patch applied cleanly
  • Token browser shows image styles as Tokens
Bambell’s picture

FileSize
38.9 KB

Hi @silkogelman. Thanks for reviewing. I'm not currently providing any replacement for tokens node:{field_name}:{style_name}. Not too familiar with Metatag module, can you please tell me exactly what replacement you expect for those tokens? I'm currently only providing replacements for :

node:{field_name}:{style_name}:filesize
node:{field_name}:{style_name}:height
node:{field_name}:{style_name}:mimetype
node:{field_name}:{style_name}:uri
node:{field_name}:{style_name}:width

Regarding "And to be completed : [...] Token browser shows image styles as Tokens", that was also done by the original D7 module :

Drupal 7 ImageCache Token image styles as tokens.

Isn't that also what you're trying to achieve with token [node:field_social_media_image:open_graph]?

Thanks.

silkogelman’s picture

Did some more testing. I tested these:

node:{field_name}:{style_name}:filesize
node:{field_name}:{style_name}:height
node:{field_name}:{style_name}:mimetype
node:{field_name}:{style_name}:uri
node:{field_name}:{style_name}:width

they all work. Awesome.

For the og:image tag I believe we need URL (node:{field_name}:{style_name}:url).
It shows up in the Token browser but in my (limited) tests it doesn't translate to the actual file url.

I was expecting [node:field_social_media_image:open_graph] to output the URL, as that was the case with imagecache_token module in D7.
Not sure what the reason for that was as node:{field_name}:{style_name}:url seems to be more fitting.
I hope others can pitch in on this matter. (update: not sure if url is supposed to contain the link to the image)

Drupal versions tested:

  • 8.1.3 (local dev)

module versions:

  • token 8.x-1.x-dev (2016-Jun-22)
  • metatag 8.x-1.0-beta9

P.S. I wasn't aware Token (browser) provides node:{field_name}:{style_name} out of the box. Always used in combination with imagecache_token module in D7, believing it provided that too. Thanks!

silkogelman’s picture

Little bit of extra info on the Metatag og:image field: (quoting the description in the module)

og:image field:
"The URL of an image which should represent the content. For best results use an image that is at least 1200 x 630 pixels in size, but at least 600 x 316 pixels is a recommended minimum. Supports PNG, JPEG and GIF formats. Should not be used if og:image:url is used. Multiple values may be used, separated by a comma. Note: Tokens that return multiple values will be handled automatically. This will be able to extract the URL from an image field."

og:image:url field:
"A alternative version of og:image and has exactly the same requirements; only one needs to be used. Multiple values may be used, separated by a comma. Note: Tokens that return multiple values will be handled automatically. This will be able to extract the URL from an image field."

So to my best knowledge we'd need
node:{field_name}:{style_name} to work with og:image (currently its not working)
node:{field_name}:{style_name}:url to work with og:image:url -> (node:{field_name}:{style_name}:url doesn't work yet)

But we need to verify that.

DamienMcKenna’s picture

Are there any plans to add "node:{field_name}:{style_name}"? That would work with Metatag because it can extract the URL from an image HTML tag (<img src="/path/to/image.png" alt="" />). "node:{field_name}:{style_name}:url" should work too. Does the Devel module on D8 have a Token tab like it used to work in D7 that could be used to compare the output?

Bambell’s picture

Thanks for all the enthusiasm about this patch. This is mostly a proof of concept right now and we're looking at first getting #2621598: Add support for field properties done, as it will affect this patch (Imagecache Token). You are very much welcome to test this other patch. Thanks.

Berdir’s picture

Status: Needs review » Needs work

#2621598: Add support for field properties is in, so we can continue to work on this issue now.

Bambell’s picture

The patch needed a re-roll after the big field properties patch, an interdiff wouldn't make to much sense here.

I added an intermediary token, a suggested earlier, to avoid possible token name conflicts :

[node:{field_name}:image_styles:{image_style}:{property}]

I spent way to much time to figure this out, but I'm almost certain that the token tree (browser) depth is limited to 4. That is, it's impossible to navigate to the ImageStyle property, starting from Node. The token browser stops at :

[node:{field_name}:image_styles:{image_style}]

Let's drop image_styles?

Berdir’s picture

Status: Needs review » Needs work
  1. +++ b/token.tokens.inc
    @@ -381,6 +382,60 @@ function token_token_info() {
    +    $image_styles = image_style_options(FALSE);
    +    foreach ($image_styles as $style => $description) {
    +      $info['tokens']['image_styles'][$style] = [
    +        'name' => $style,
    +        'description' => t('@description image.', ['@description' => $description]),
    +        'type' => 'image_style',
    +      ];
    +    }
    
    @@ -1244,6 +1348,14 @@ function field_token_info_alter(&$info) {
    +      // For fields of type image, the field's type is changed and field
    +      // properties are not added.
    +      if (($field->getType() == 'image') && \Drupal::moduleHandler()->moduleExists('image')) {
    +        $info['tokens'][$token_type][$field_name]['type'] = 'image_field';
    +        continue;
    +      }
    

    As discussed, instead of changing the token type, we want to add the image style tokens to image field type token types (what a word).

    Also, now that image_style as token type is gone, we need to define it ourself.. and try to avoid using image_style, due to the possible clash with the entity type. A bit long, but maybe simply image_with_image_style?

  2. +++ b/token.tokens.inc
    @@ -436,6 +491,55 @@ function token_tokens($type, array $tokens, array $data = array(), array $option
    +      $parts = explode(':', $name);
    +      if ($node->hasField($parts[0]) && \Drupal::moduleHandler()->moduleExists('image')) {
    +        $field = $node->get($parts[0]);
    +        $image_styles = image_style_options(FALSE);
    +        if ((count($parts) == 4) && array_key_exists($parts[2], $image_styles) && ($field->getFieldDefinition()->getType() == 'image')) {
    

    this logic should now be moved into the new part below "elseif (!empty($data['field_property'])) {"., around line 1411.

    The field detection at that point already happend, so you should be able to just use that, as the code there does for the real properties.

Bambell’s picture

Here we go. The image style tokens are now added to the appropriate field, so that the tokens for the field's properties are preserved. The tokens for the image style properties are now of type image_with_image_style. I left the intermediary token out. Perhaps we could append a prefix to the image style token name? For instance "image-style-thumbnail" instead if "thumbnail"?

Still need to address #20, #21 and #22 about a URL token.

Berdir’s picture

Status: Needs review » Needs work

Getting closer.

  1. +++ b/token.tokens.inc
    @@ -1277,6 +1310,18 @@ function field_token_info_alter(&$info) {
    +
    +      // Provide image_with_image_style tokens for image fields.
    +      if (($field->getType() == 'image') && \Drupal::moduleHandler()->moduleExists('image')) {
    +        $image_styles = image_style_options(FALSE);
    

    the field type image can't really exist without the module also being enabled, so I think just checking for the field type is enough here.

  2. +++ b/token.tokens.inc
    @@ -1422,6 +1467,54 @@ function field_tokens($type, $tokens, array $data = array(), array $options = ar
    +
    +      // Handle [field_name:image_style:token] tokens.
    +      if (\Drupal::moduleHandler()->moduleExists('image')) {
    +        $field = $data[$data['field_name']];
    +        $parts = explode(':', $name);
    +        $image_styles = image_style_options(FALSE);
    

    same here, move the field type check into the first if instead. Otherwise you end up preparing the image styles for every call that gets here, even if it's not an image field. and then the module check is not needed.

  3. +++ b/token.tokens.inc
    @@ -1422,6 +1467,54 @@ function field_tokens($type, $tokens, array $data = array(), array $options = ar
    +          if (!file_exists($derivative_uri)) {
    +            // If the image derivative already exists, don't re-generate it.
    +            $derivative_exists = $style->createDerivative($original_uri, $derivative_uri);
    +          }
    +          else {
    +            $derivative_exists = TRUE;
    +          }
    +
    

    as discussed, we need to implement the optimization to only create/load the derivative if necessary. shoudn't be for uri, url (which is missing), width/height.

Bambell’s picture

There. Token replacement should be as optimized as it can be. I also added url tokens, so it should be ready for testing at this point. [node:{field_name}:{image_style}] has no replacement, for urls, :url token has to be used for now.

Berdir’s picture

+++ b/token.tokens.inc
@@ -1469,50 +1469,61 @@ function field_tokens($type, $tokens, array $data = array(), array $options = ar
 
-        if ((count($parts) == 2) && array_key_exists($parts[0], $image_styles) && ($field->getFieldDefinition()->getType() == 'image')) {
-          // An image derivative of the original has to be created first.
+        if ((count($parts) == 2) && array_key_exists($parts[0], $image_styles)) {
+          $style_name = $parts[0];
...
           /** @var \Drupal\file\Entity\File $field_image */

Unless I'm missing something, supporting [node:field_name:image_style] should be fairly easy:

Just change this to count($parts) == 1 && array_key_exists() and then do:

$derivative_property = isset($parts[1]) ? $parts[1] : 'url';

Looks pretty good to me otherwise. Will need to do another full review. And now it is time to do some testing with this :)

Bambell’s picture

Ho, I was just not entirely certain if that should be supported or not.

Bambell’s picture

Adding tests for [node:{field_name}:{image_style}] tokens.

Berdir’s picture

Title: Integrate Imagecache Token (D7) module » Add image style tokens for image fields
Issue summary: View changes

Updating the issue title.

Berdir’s picture

Status: Needs review » Needs work
  1. +++ b/src/Tests/TokenFieldUiTest.php
    @@ -94,4 +108,112 @@ class TokenFieldUiTest extends TokenTestBase {
    +      'field_image:thumbnail:filesize' => $image_1_thumbnail->getFileSize(),
    +      'field_image:medium:filesize' => $image_1_medium->getFileSize(),
    +      'field_image:large:filesize' => $image_1_large->getFileSize(),
    +      'field_image:thumbnail:height' => $image_1_thumbnail->getHeight(),
    +      'field_image:medium:height' => $image_1_medium->getHeight(),
    +      'field_image:large:height' => $image_1_large->getHeight(),
    +      'field_image:thumbnail:width' => $image_1_thumbnail->getWidth(),
    +      'field_image:medium:width' => $image_1_medium->getWidth(),
    +      'field_image:large:width' => $image_1_large->getWidth(),
    +      'field_image:thumbnail:uri' => $style_thumbnail->buildUri('public://example1.png'),
    

    Dynamic tests like this tend to be hard to follow because you can't easily see the expected values.

    We're also not testing whether those tokens work when the files don't exist yet.
    at least height/width should be predictable, not sure about filesize but that should also always result in the same value?

  2. +++ b/token.tokens.inc
    @@ -383,6 +384,38 @@ function token_token_info() {
    +    ];
    +    $info['tokens']['image_with_image_style']['uri'] = [
    +      'name' => t('URI'),
    +      'description' => t('The URI to the image.'),
    

    I'm missing a token info entry for the url.

    Also noticed that we don't have test coverage for this. See what I did for the entity reference patch, get the token info and check that the token type has some of those tokens and that the image field token types have the image style tokens.

  3. +++ b/token.tokens.inc
    @@ -1277,6 +1310,18 @@ function field_token_info_alter(&$info) {
    +
    +      // Provide image_with_image_style tokens for image fields.
    +      if ($field->getType() == 'image') {
    +        $image_styles = image_style_options(FALSE);
    

    I was wondering if we could also define the image styles on the file token type, then we only need them on a single place. Of course, then the UI will definitely not work anymore right now, so lets keep this.

  4. +++ b/token.tokens.inc
    @@ -1277,6 +1310,18 @@ function field_token_info_alter(&$info) {
    +            'name' => $style,
    

    name is the user-facing label, that should be the image style label ($description). The description could say that this represents the image in the given image style or so.

  5. +++ b/token.tokens.inc
    @@ -1422,6 +1467,67 @@ function field_tokens($type, $tokens, array $data = array(), array $options = ar
    +      // Handle [field_name:image_style:token] tokens.
    +      $field = $data[$data['field_name']];
    +      if ($field->getFieldDefinition()->getType() == 'image') {
    +        $parts = explode(':', $name);
    +        $image_styles = image_style_options(FALSE);
    

    we already have the $field_item available now and we have the $property_name that we can use as well.

    Also, it would be more correct to generate those tokens like we do entity reference tokens now, by finding nested tokens and them passing them alongg with the necessary data (file + width/height I suppose).

    token types should be re-usable in different contexts, this now only works within an image field.

  6. +++ b/token.tokens.inc
    @@ -1422,6 +1467,67 @@ function field_tokens($type, $tokens, array $data = array(), array $options = ar
    +
    +          // Only generate the image derivative if needed.
    +          if ($derivative_property === 'width' || $derivative_property === 'height') {
    +            /** @var \Drupal\Core\Image\Image $image */
    +            $image = \Drupal::service('image.factory')->get($original_uri);
    +            $variables = [
    +              'width' => $image->getWidth(),
    +              'height' => $image->getHeight(),
    +              'style_name' => $style_name,
    +              'uri' => $original_uri,
    +              'attributes' => [],
    +            ];
    +            template_preprocess_image_style($variables);
    +            $replacements[$original] = $variables['image']["#$derivative_property"];
    +          }
    

    that's a bit too much code-reuse and I don't think we actually end up with fewer lines.

    You also still load the image for the original width/height, you have this information already on $field_item

    You load the style below anyway. And instead of loading them all and then checking if this exists, we could actually just do if ($style = ImageStyle::load($property_name).

    Then, instead of building that array, just copy the 4 lines from that function that we need and call transformDimensions() yourself.

Bambell’s picture

Dynamic tests like this tend to be hard to follow because you can't easily see the expected values.

We're also not testing whether those tokens work when the files don't exist yet.
at least height/width should be predictable, not sure about filesize but that should also always result in the same value?

I left this for now, but will hardcode test values in a next patch. Will also look into tests without a file uploaded, but I'm almost certain the file size will vary.

I was wondering if we could also define the image styles on the file token type [...].

This was done in the Drupal 7 Imagecache Token module, if I recall correctly.

Other points have been addressed. I also added test coverage for multivalued image fields.

Bambell’s picture

Status: Needs work » Needs review
Bambell’s picture

Patch with hardcoded values in tests, hopefully will pass here.

Status: Needs review » Needs work

The last submitted patch, 37: add_image_style_tokens-2611866-37.patch, failed testing.

The last submitted patch, 37: add_image_style_tokens-2611866-37.patch, failed testing.

Bambell’s picture

Changing hardcoded test values so that test passes.

Berdir’s picture

Status: Needs review » Needs work
+++ b/src/Tests/TokenFieldUiTest.php
@@ -94,4 +119,147 @@ class TokenFieldUiTest extends TokenTestBase {
+      'field_image:thumbnail:filesize' => '6909',

as discussed, lets go back to getting just the file size from the style object.

And delete the style files after defining $tokens again, so that we can test the if not exists condition.

Bambell’s picture

Assigned: Unassigned » Bambell
Status: Needs work » Needs review
FileSize
18.33 KB
8.34 KB

It works out of the box (after deleting the image files) for everything but the MIME type and file size, yay !

$derivative_exists = TRUE;
if (!file_exists($derivative_uri)) {
  $derivative_exists = $style->createDerivative($original_uri, $derivative_uri);
}
if ($derivative_exists) {
  // MIME type and file size...
}

And createDerivative() should gracefully return false.

Berdir’s picture

Status: Needs review » Needs work
+++ b/src/Tests/TokenFieldUiTest.php
@@ -239,6 +246,69 @@ class TokenFieldUiTest extends TokenTestBase {
+    // Delete the image files and assert that the width, height, uri and url
+    // tokens are still usable.
+    foreach (File::loadMultiple() as $file) {
+      $file_name = $file->getFilename();
+      if (($file_name === 'example1.png') || ($file_name === 'example2.gif')) {
+        $file->delete();
+      }
+    }

This is not what I meant :)

Only delete the derivatives (already for the first test), so they have to be regenerated and work even when they do not exist yet.

Not sure if this is a scenario worth testing, it's not really supposed to exist, but I guess making sure that it doesn't fail is OK.

delalis’s picture

Hey guys (@Bambell and @Berdir), I just wanted to say thank you for working so hard on this issue. It is an important one for social media image sharing on everyones site and as a follower of this thread, I'm excited to see it merged in. Sorry I don't have the time to contribute but you guys seem nearly ready to go.

Cheers and thanks again! #EndMotivationalSpeech

Bambell’s picture

Here we go, deleting the image derivatives before asserting the tokens.

Berdir’s picture

Status: Needs review » Fixed

Nice, committed :)

  • Berdir committed 76db64a on 8.x-1.x authored by Bambell
    Issue #2611866 by Bambell: Add image style tokens for image fields
    
delalis’s picture

Just tested it by updating my Token module to the latest dev version and it works like a charm. Seriously, thanks again guys!!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.