It is not reasonable to use debug_backtrace in production code. menu_get_item will return global context that is easier to parse.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dave Reid’s picture

Status: Needs review » Needs work

menu_get_item() would result in a significant regression because it would not support Panels and Page Manager, as well as outputting individual fields using field_view_field().

greg.1.anderson’s picture

Status: Needs work » Needs review
FileSize
4.32 KB

While I certainly see the point of #1, I still cannot abide running debug_backtrace() in production code. At the same time, it would not do to introduce such a large regression, particularly when there are significant numbers of people who do not share that feeling.

Perhaps this patch might be a suitable compromise.

* menu_get_item() is still used in the primary handler to find the node or taxonomy term of the current menu item, as applicable, as it is not the introduction of menu_get_item(), but the removal of debug_backtrace() that introduces the regression in behavior. (Note also that this version of the patch fixes a serious problem in #0 -- $data['user'] should not be assigned the user from menu_get_item(), because that would return the user being viewed on the current user page, and it would not do to have the token [user:name] sometimes refer to the logged-in user, and sometimes review to the user on the page being viewed.)

* A call to drupal_alter() is introduced, so modules such as Page Manger and Panels can add information to $data and $options based on context in some future release (if desired).

* For backwards compatibility with the existing release of token_filter, the special code that uses debug_backtrace() is moved to a submodule, token_filter_advanced(), that implements the appropriate hook to fill in $data and $options, exactly as it did before.

* A hook_update_N() is also provided to enable token_filter_advanced for all current users of token_filter, so that the capabilities of the module do not suddenly change due to a minor update. New users of the module must explicitly enable token_filter_advanced.

chinhlc’s picture

jpoesen’s picture

Hi Dave. As a response to #1 I'm finding that: field_view_field() does not actually trigger token replacement, whereas field_attach_view() does.
More info: https://www.drupal.org/node/2151127#comment-9117837.

Dave Reid’s picture

@jpoesen: Just following the logic, I can't say that it's true. Both field_view_field() and field_attach_view() call the following line:

$output = _field_invoke_default('view', $entity_type, $entity, $view_mode, $null, $options);

Which is translated to calling field_default_view(). I'm not sure why it wouldn't be happening in your case.

jpoesen’s picture

@davereid: thanks, I've responded in the other thread. My comment in #4 shouldn't have been in this thread.

heddn’s picture

Status: Needs review » Reviewed & tested by the community

Marking as RTBCed. This actually helps in a case I've got with Panels and Views where I've overridden /taxonomy/term/%taxonomy_term in a panel page. Since the "description" field on the term is actually a property, the backtrace doesn't include field_default_view and the token term name substitution fails.

Dave Reid’s picture

Status: Reviewed & tested by the community » Needs review

I'm still not sold on this approach, so I need to think about this more. For instance, if this was rendering a taxonomy term from a node view, this would also cause the tokens in the term description to not work, as it would only replace the node tokens.

greg.1.anderson’s picture

The latest suggested patch (#2) should behave the same as the current code; it just gives folks the option to turn of the hook that contains debug_backtrace by disabling the new module added in the patch. Those who chose to disable the new module would have problems per #8, but by default, the new module is enabled by the update hook, so existing users should be unaffected.

heddn’s picture

re: #8
I'd really like to see both approaches available. For my use case in #7, backtrace doesn't work. But the patch in #2 does. With the drupal_alter, this gives us all kinds of possibilities.

heddn’s picture

Updated patch #2 to address a problem with the default 'node' menu item.

darvanen’s picture

Status: Needs review » Closed (duplicate)
Related issues: +#2075663: Improve context logic for filter processing.

This would be fixed by the approach in #2075663: Improve context logic for filter processing. if it gets included.

Valid concerns here about Panels and Page Manager which I will copy over to that issue.