Problem/Motivation

We are eliminating the process layer from Drupal 8, since the reasons for it's existence can now be solved in different ways. One of the functions on the chopping block is template_process_username(). Since all this function does is add attributes, now a renderable Attribute object, this variable can safely be moved to the preprocess layer.

<?php
function template_process_username(&$variables) {
 
// Finalize the link_options array for passing to the l() function.
  // This is done in the process phase so that attributes may be added by
  // modules or the theme during the preprocess phase.
 
if (isset($variables['link_path'])) {
   
// $variables['attributes'] contains attributes that should be applied
    // regardless of whether a link is being rendered or not.
    // $variables['link_attributes'] contains attributes that should only be
    // applied if a link is being rendered. Preprocess functions are encouraged
    // to use the former unless they want to add attributes on the link only.
    // If a link is being rendered, these need to be merged. Some attributes are
    // themselves arrays, so the merging needs to be recursive.
   
$variables['link_options']['attributes'] = array_merge_recursive($variables['link_attributes'], $variables['attributes']);
  }
}
?>

Proposed resolution

Move $variables['link_options']['attributes'] or $variables['attributes'] into the preprocess layer.
- remove this function.

Remaining tasks

None.

User interface changes

None.

API changes

None.

#1843650: Remove the process layer (hook_process and hook_process_HOOK)
#1833920: [META] Markup Utility Functions
#1836730: Add a renderable object that is equivalent to l()
#1898468: user.module - Convert PHPTemplate templates to Twig

Files: 
CommentFileSizeAuthor
#34 1843788_34_template_process_username.patch716 bytesEric_A
PASSED: [[SimpleTest]]: [MySQL] 57,662 pass(es).
[ View ]
#30 1843788_30_template_process_username.patch5.58 KBscor
PASSED: [[SimpleTest]]: [MySQL] 57,600 pass(es).
[ View ]
#27 1843788_27_template_process_username.patch5.59 KBscor
PASSED: [[SimpleTest]]: [MySQL] 55,442 pass(es).
[ View ]
#22 1843788_22_template_process_username.patch5.63 KBscor
PASSED: [[SimpleTest]]: [MySQL] 56,028 pass(es).
[ View ]
#22 interdiff.txt1.15 KBscor
#16 1843788_16_template_process_username.patch5.4 KBscor
FAILED: [[SimpleTest]]: [MySQL] 56,331 pass(es), 9 fail(s), and 5,838 exception(s).
[ View ]
#13 1843788_13_template_process_username.patch5.4 KBscor
FAILED: [[SimpleTest]]: [MySQL] 56,880 pass(es), 8 fail(s), and 5,836 exception(s).
[ View ]
#8 drupal.remove_template_process_username.1843788.08.patch1.46 KBtyjamessmith
FAILED: [[SimpleTest]]: [MySQL] Repository checkout: failed to checkout from [git://git.drupal.org/project/drupal.git].
[ View ]
#1 core-remove_template_process_username-1843788-1.patch931 bytesjenlampton
FAILED: [[SimpleTest]]: [MySQL] 55,922 pass(es), 5 fail(s), and 0 exception(s).
[ View ]

Comments

jenlampton’s picture

Status:Active» Needs review
StatusFileSize
new931 bytes
FAILED: [[SimpleTest]]: [MySQL] 55,922 pass(es), 5 fail(s), and 0 exception(s).
[ View ]

This will need to be revisited after #attributes get pulled out of link_options #1836730: Add a renderable object that is equivalent to l()
But for now... patch needs review.

Status:Needs review» Needs work
Issue tags:-Twig, -theme system cleanup

The last submitted patch, core-remove_template_process_username-1843788-1.patch, failed testing.

joelpittet’s picture

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, core-remove_template_process_username-1843788-1.patch, failed testing.

mariagwyn’s picture

Status:Needs work» Needs review

Status:Needs review» Needs work
Issue tags:+Twig, +theme system cleanup

The last submitted patch, core-remove_template_process_username-1843788-1.patch, failed testing.

tyjamessmith’s picture

Assigned:Unassigned» tyjamessmith

Working on making a new patch

tyjamessmith’s picture

Status:Needs work» Needs review
StatusFileSize
new1.46 KB
FAILED: [[SimpleTest]]: [MySQL] Repository checkout: failed to checkout from [git://git.drupal.org/project/drupal.git].
[ View ]

Patch needs review

socketwench’s picture

+++ b/core/modules/user/user.moduleundefined
@@ -764,14 +762,7 @@ function template_preprocess_username(&$variables) {
-function template_process_username(&$variables) {
+  ¶
   // Finalize the link_options array for passing to the l() function.
   // This is done in the process phase so that attributes may be added by
   // modules or the theme during the preprocess phase.
@@ -806,7 +797,6 @@ function template_process_username(&$variables) {

@@ -806,7 +797,6 @@ function template_process_username(&$variables) {
  *     Drupal\Core\Template\Attribute class if not linking to the user's page.
  *
  * @see template_preprocess_username()
- * @see template_process_username()
  */
function theme_username($variables) {
   if (isset($variables['link_path'])) {

There's a whitespace issue, but otherwise it looks fine.

socketwench’s picture

Status:Needs review» Needs work
tyjamessmith’s picture

Status:Needs work» Needs review
Issue tags:-Twig, -theme system cleanup

Status:Needs review» Needs work
Issue tags:+Twig, +theme system cleanup

The last submitted patch, drupal.remove_template_process_username.1843788.08.patch, failed testing.

scor’s picture

Status:Needs work» Needs review
StatusFileSize
new5.4 KB
FAILED: [[SimpleTest]]: [MySQL] 56,880 pass(es), 8 fail(s), and 5,836 exception(s).
[ View ]

The reason we have a template_process_username is because we use an intermediary 'link_attributes' to carry around attributes, but these attributes can directly be placed in ['link_options']['attributes']. That's what this patch does.

Status:Needs review» Needs work
Issue tags:-Twig, -theme system cleanup

The last submitted patch, 1843788_13_template_process_username.patch, failed testing.

tim.plunkett’s picture

Status:Needs work» Needs review
Issue tags:+Twig, +theme system cleanup
scor’s picture

StatusFileSize
new5.4 KB
FAILED: [[SimpleTest]]: [MySQL] 56,331 pass(es), 9 fail(s), and 5,838 exception(s).
[ View ]

keeping up with HEAD

Status:Needs review» Needs work
Issue tags:-Twig, -theme system cleanup

The last submitted patch, 1843788_16_template_process_username.patch, failed testing.

joelpittet’s picture

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, 1843788_16_template_process_username.patch, failed testing.

scor’s picture

Status:Needs work» Needs review

#16: 1843788_16_template_process_username.patch queued for re-testing.

git apply --check -p1 1843788_16_template_process_username.patch works for me on both 0bed623 and the latest HEAD (c4e52bb)

Status:Needs review» Needs work
Issue tags:+Twig, +theme system cleanup

The last submitted patch, 1843788_16_template_process_username.patch, failed testing.

scor’s picture

Status:Needs work» Needs review
StatusFileSize
new1.15 KB
new5.63 KB
PASSED: [[SimpleTest]]: [MySQL] 56,028 pass(es).
[ View ]

$variables['attributes'] needed to be initialized.

Status:Needs review» Needs work
Issue tags:-Twig, -theme system cleanup

The last submitted patch, 1843788_22_template_process_username.patch, failed testing.

scor’s picture

Status:Needs work» Needs review
Issue tags:+Twig, +theme system cleanup

#22: 1843788_22_template_process_username.patch queued for re-testing.

HEAD was broken when this patch was first submitted. It has been fixed in the meantime.

joelpittet’s picture

Assigned:tyjamessmith» Unassigned
Status:Needs review» Reviewed & tested by the community

Great work, thanks @scor

Cottser’s picture

Status:Reviewed & tested by the community» Needs work
Issue tags:+Needs reroll

Needs a reroll.

scor’s picture

Status:Needs work» Needs review
StatusFileSize
new5.59 KB
PASSED: [[SimpleTest]]: [MySQL] 55,442 pass(es).
[ View ]

no problem.

Cottser’s picture

Status:Needs review» Reviewed & tested by the community

Reroll looks good, thanks! Back to RTBC per #25.

alexpott’s picture

Status:Reviewed & tested by the community» Needs work

Needs a reroll

curl https://drupal.org/files/1843788_27_template_process_username.patch | git a
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  5729  100  5729    0     0   6113      0 --:--:-- --:--:-- --:--:--  7568
error: patch failed: core/modules/node/node.module:1084
error: core/modules/node/node.module: patch does not apply
scor’s picture

Status:Needs work» Needs review
StatusFileSize
new5.58 KB
PASSED: [[SimpleTest]]: [MySQL] 57,600 pass(es).
[ View ]
scor’s picture

Status:Needs review» Reviewed & tested by the community

back to RTBC per #28.

Cottser’s picture

Issue tags:-Needs reroll

Yup, looks good. Thanks @scor!

alexpott’s picture

Status:Reviewed & tested by the community» Fixed

Committed fa9c197 and pushed to 8.x. Thanks!

Eric_A’s picture

Priority:Normal» Major
Status:Fixed» Needs review
StatusFileSize
new716 bytes
PASSED: [[SimpleTest]]: [MySQL] 57,662 pass(es).
[ View ]

By initialising attributes in preprocess this broke the ability to pass attributes to the theme layer.
I fear this is something that is all over core. (In fact, there is the 'extra' variable right here in template_preprocess_username().)

Here's a patch to fix the regression at hand. There's hook_theme() to safely initialise theme variables.

joelpittet’s picture

Status:Needs review» Reviewed & tested by the community

Thank you @Eric_A!

joelpittet’s picture

Issue tags:+Quick fix

tagging

alexpott’s picture

Status:Reviewed & tested by the community» Fixed

Committed 757d39e and pushed to 8.x. Thanks!

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

Anonymous’s picture

Issue summary:View changes

Added twig user conversion