Commits from token 7.x-1.7

[jenlampton]

• Issue #1821902: Fixed false positives in recursive token check in _token_build_tree() backdrop#4487 | d04c7b8 | Issue #1821902 by Antonin Brunon, emattias: Fixed palse positives in recursive token check in _token_build_tree() for tokens that had a name of '0' (e.g. entity_token module's delta '0' field tokens).
• Fixed tokens without a description value were treated as wrong. backdrop#4488 | 2187bd6 | Fixed tokens without a description value were treated as wrong.
• Issue #5014: Soften token status report messages, split into individual items with different severity values. backdrop#4489 | b189c11 | Soften token status report messages, split into individual items with different severity values.
• Issue #2299039: Remove unused $sanitize variable from field_tokens(). backdrop#4490 | e6c2702 | Issue #2299039 by Elijah Lynn: Remove unused $sanitize variable from field_tokens().
• Issue #2485221: token_pre_render_field_token() parameter should not be passed by reference backdrop#4491 | 4c39c41 | Issue #2485221 by Berdir: Fixed token_pre_render_field_token() parameter should not be passed by reference.
• N/A??? - see comment below | 0a620c8 | Issue #2474403 by kfritsche, mariamania, Zekvyrin, B-Prod: Fixed token replacement of field descriptions should not replace i18n_field.module's translations.
• Issues #2630378 & #2702379: Fix messages in token_element_validate() backdrop#4493 | d9f4e54 | Issue #2630378 by hussainweb: Token "cannot contain fewer than one token" error misleading
• Issues #2630378 & #2702379: Fix messages in token_element_validate() backdrop#4493 | cf53b40 | Issue #2702379 by Dave Reid: Review @todos in token_element_validate()
• N/A - already addressed in a different way in tokens list not shown on url path patterns page #2697 | 6fa49e9 | Issue #2825841 by hbemtec: Fixed PHP 7.1 compatibility error: Cannot use string offset as an array in _token_token_tree_format_row().

This list generated with (and then d.org issue links manually added):
git log 7.x-1.6..7.x-1.7 --reverse --no-merges --pretty=format:"- [ ] #xx | [%h](https://git.drupalcode.org/project/token/-/commit/%H) | [%s](https://drupal.org/node/)"

[klonos]

PRs:

• Issue #1821902: Fixed false positives in recursive token check in _token_build_tree() backdrop#4487
• Fixed tokens without a description value were treated as wrong. backdrop#4488
• Issue #5014: Soften token status report messages, split into individual items with different severity values. backdrop#4489
• Issue #2299039: Remove unused $sanitize variable from field_tokens(). backdrop#4490
• Issue #2485221: token_pre_render_field_token() parameter should not be passed by reference backdrop#4491
• Issues #2630378 & #2702379: Fix messages in token_element_validate() backdrop#4493

[klonos]

With re to https://drupal.org/node/2474403 and the respective commit:

function token_field_widget_form_alter(&$element, &$form_state, $context) {
  if (!empty($element['#description']) && !empty($context['instance']['description'])) {
-    $element['#description'] = filter_xss_admin(token_replace($context['instance']['description']));
+    $instance = $context['instance'];
+    if (module_exists('i18n_field')) {
+      $instance = i18n_string_object_translate('field_instance', $instance);
+    }
+    $element['#description'] = field_filter_xss(token_replace($instance['description']));
  }
}

This is with regards to a contrib module, and AFAIK we don't account for contrib in core that way. Also, that function doesn't exist in Backdrop core, and I wasn't able to find any respective/equivalent. Should that issue/commit be marked as N/A then? Is there a way for this to be overridden and fixed in i18n_field?

[argiepiano]

@klonos please see my comment in the PR queue (I decided to comment there, since this issue contains many PRs)

[klonos]

Thanks @argiepiano 🙏🏼 ...yes, this is how we work on these cross-port issues (I mean that we create multiple PRs linked to the same issue, and we review each PR in its own thread instead of the issue thread as we'd normally do for other issues). It would be impractical to be opening separate issues for each commit, as there are sometimes dozens of them in each cross-port issue. What we often do is to break up the commit history (if we are really behind), and we batch certain versions/tags together, to what seems like a manageable amount of commits each time.

Having said that, there might be cases where we break a specific commit out into its own issue, if it is a significant change that warrants separate discussion/decision. I don't recall doing that many times in the past for other crossports though. It's just a rare thing.

Another thing to keep in mind is that sometimes we may have crossported a fix independently of the crossport issue, at some time before the crossport issue got created. In that case, we simply link that issue in the issue summary of the crossport issue, and mark the respective commit as done (tick the checkbox next to it in the list).

Anyway, back to the issue you mention in the PR, I've responded in there 👍🏼 ...happy to mark it as N/A, however see my comment re code parity when possible (or "harmless" as you said).

[klonos]

6fa49e9 | Issue #2825841 by hbemtec: Fixed PHP 7.1 compatibility error: Cannot use string offset as an array in _token_token_tree_format_row(). is N/A to Backdrop, since we are already initializing the various relevant values in $defaults as arrays rather than strings earlier in the _token_token_tree_format_row() function. Our code:

  static $defaults = array(
    'id' => '',
    'class' => array(),
    'data' => array(
      'name' => array(),
      'token' => array(),
      'value' => '',
    ),
  );

vs. the 7.x code:

  static $defaults = array(
    'id' => '',
    'class' => array(),
    'data' => array(
      'name' => '',
      'token' => '',
      'value' => '',
      'description' => '',
    ),
  );

It has been fixed since 1.7.1 (back in May 2017), in #2697 (commit).

[klonos]

...OK, I'm done here. Moving to #5013 next.

Please review each 7.x commit in the respective PR (links in the summary and the sidebar), and remember to leave a comment here when you have added any comment in any of the PRs. Thanks 🙏🏼

[argiepiano]

I'd like to suggest that you combine backdrop/backdrop#4488 and backdrop/backdrop#4489 into one PR. They both change some of the same lines.

[klonos]

Thanks @argiepiano ...yes, as I mentioned in another comment on the PR, I have already done that in backdrop/backdrop#4493, where I had to merge the changes of 3 commits from 7.x.

[argiepiano]

Regarding the comment above:

This is with regards to a contrib module, and AFAIK we don't account for contrib in core that way. Also, that function doesn't exist in Backdrop core, and I wasn't able to find any respective/equivalent. Should that issue/commit be marked as N/A then? Is there a way for this to be overridden and fixed in i18n_field?

The D7 problem was that, on a site with Field Translation enabled, as soon as you enabled Token, the field descriptions were not translated anymore. I've tested enabling Field Translation with Backdrop core (which includes the Token functionality), and this is not happening, so this patch is indeed not needed.

However, there is an unrelated bug that prevents the use of tokens in field descriptions. Tokens are simply not replaced. Despite an initial replacement done in this line, the replacement is later reverted.

[indigoxela]

AFAIK we don't account for contrib in core that way.

@klonos Agreed, this module specific code doesn't belong to core. And seems to be unnecessary, anyway.

[argiepiano]

@klonos, wondering if it may be a good idea to create issues for each PR. Right now there is this main issue in backdrop-issues, and a bunch of PRs in backdrop. By creating issues for each item in the list, we can mark each as "tested" or "WFM", and eventually merge only those that have been reviewed and tested. Right now there is no place to indicate that a specific PR WFM other than putting that in the PR request itself.