Problem/Motivation
Based on
🐛
Notice: Undefined offset: 2 in _contextual_id_to_links()
Fixed
I'd like to suggest implementing a pre-check of the expected contextual link format (needs exactly three strings separated by ":").
I once again saw a wrong implementation where the format was wrong (only one ":" used) which leads to the following core warning:
Warning: Undefined array key 2 in _contextual_id_to_links() (Zeile 211 in x/web/core/modules/contextual/contextual.module).
and it's hard to find the place, where it was implemented wrong in the theme.
As @Chi said in
#18
🐛
Notice: Undefined offset: 2 in _contextual_id_to_links()
Fixed
in the other issue
In Drupal core it does not cause any problems as these ID is generated automatically.
So as twig_tweak is exposing this function to twig, it would make sense to pre-check the required format and help users to track down issues with wrong format.
Steps to reproduce
Use {{ drupal_contextual_links('a:b:c') }} with a wrong format, typically without the second ":".
Proposed resolution
Trigger a warning about the wrong format, if the parameter only
For comparison, see contextual.module:
/**
* Unserializes the result of _contextual_links_to_id().
*
* Note that $id is user input. Before calling this method the ID should be
* checked against the token stored in the 'data-contextual-token' attribute
* which is passed via the 'tokens' request parameter to
* \Drupal\contextual\ContextualController::render().
*
* @param string $id
* A serialized representation of a #contextual_links property value array.
*
* @return array
* The value for a #contextual_links property.
*
* @see _contextual_links_to_id()
* @see \Drupal\contextual\ContextualController::render()
*/
function _contextual_id_to_links($id) {
$contextual_links = [];
$contexts = explode('|', $id);
foreach ($contexts as $context) {
[$group, $route_parameters_raw, $metadata_raw] = explode(':', $context);
parse_str($route_parameters_raw, $route_parameters);
$metadata = [];
parse_str($metadata_raw, $metadata);
$contextual_links[$group] = [
'route_parameters' => $route_parameters,
'metadata' => $metadata,
];
}
return $contextual_links;
}
which just expects the "$context" to be correct.
As the current implementation in twig_tweak is quite simple (render array), we'd have to duplicate some of the logic to explode $id by "|" and ":" so it's to be discussed, if it wouldn't be even better to add that check to core to safeguard the render array in general, even if used somewhere else.
Current twig_tweak implementation:
/**
* Builds contextual links.
*
* @param string $id
* A serialized representation of a #contextual_links property value array.
*
* @return array
* A renderable array representing contextual links.
*
* @see https://www.drupal.org/node/2133283
*/
public static function drupalContextualLinks(string $id): array {
$build['#cache']['contexts'] = ['user.permissions'];
if (\Drupal::currentUser()->hasPermission('access contextual links')) {
$build['#type'] = 'contextual_links_placeholder';
$build['#id'] = $id;
}
return $build;
}
Remaining tasks
- Decide if this should be fixed in twig_tweak or core
- Implement
- Test
- Release
User interface changes
API changes
Data model changes