Text overlay tokens: pass image context into token replacement [#2702265]

When using the text overlay effect, we can use tokens. During that token replacement, there are various opportunities for other modules to provide and alter the token replacements with hook_tokens() and hook_tokens_alter().

It would be nice to pass all the context we have into the token replacement process, to give other modules an opportunity to make the best token replacements as possible.

Comment	File	Size	Author
#5	2702265-image-context-for-tokens-5.patch	1.75 KB	a_khalipau
#2	2702265-image-context-for-tokens-2.patch	2.08 KB	jfrederick

Comments

Comment #1

7 April 2016 at 21:05

jfrederick created an issue. See original summary.

Comment #2

jfrederick commented 7 April 2016 at 21:09

Status:

Active

» Needs review

Status	File	Size
new	2702265-image-context-for-tokens-2.patch	2.08 KB

And a patch

Comment #3

mondrake

🇮🇹

commented 8 April 2016 at 06:44

Tokens are not my favorite playground, so I hope @Berdir or @slashrsm can review this.

The one thing that jumps out to me: the token replacement is done in the ::getTextWrapper method for a reason - it is called by both ::applyEffect and ::transformDimensions.

Now, ::applyEffect has an image object in input, but ::transformDimension has not - because the latter is meant to precalculate derivative image dimensions without the IO load that fetching an image object brings.

So, potentially this patch introduces the risk that the two methods will return different dimensions, which we should avoid.

An option could be, instead of passing the image object to the token context, to pass an associative array with the elements that may be used by the token system, like e.g. height, width, uri - and that to be taken from the image object in ::applyEffect, and from the passed arguments in ::transformDimensions.