Remove template_process_username() [#1843788]

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.

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

Comment	File	Size	Author
#34	1843788_34_template_process_username.patch	716 bytes	Eric_A
#34
#30	1843788_30_template_process_username.patch	5.58 KB	scor
#30
#27	1843788_27_template_process_username.patch	5.59 KB	scor
#27
#22	1843788_22_template_process_username.patch	5.63 KB	scor
#22
#22	interdiff.txt	1.15 KB	scor
#16	1843788_16_template_process_username.patch	5.4 KB	scor
#16
#13	1843788_13_template_process_username.patch	5.4 KB	scor
#13
#8	drupal.remove_template_process_username.1843788.08.patch	1.46 KB	tyjamessmith
#8
#1	core-remove_template_process_username-1843788-1.patch	931 bytes	jenlampton
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

jenlampton

she / her

CreditAttribution: jenlampton commented 18 November 2012 at 04:18

Status:

Active

» Needs review

File	Size
core-remove_template_process_username-1843788-1.patch	931 bytes

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.

Comment #2

18 November 2012 at 05:11

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.

Comment #3

joelpittet

English

Vancouver

CreditAttribution: joelpittet commented 2 April 2013 at 16:49

Status:

Needs work

» Needs review

#1: core-remove_template_process_username-1843788-1.patch queued for re-testing.

Comment #4

2 April 2013 at 18:06

Status:

Needs review

» Needs work

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

Comment #5

mariagwyn CreditAttribution: mariagwyn commented 24 May 2013 at 17:16

Status:

Needs work

» Needs review

#1: core-remove_template_process_username-1843788-1.patch queued for re-testing.

Comment #6

24 May 2013 at 18:24

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.

Comment #7

tyjamessmith CreditAttribution: tyjamessmith commented 24 May 2013 at 20:01

Assigned:

Unassigned

» tyjamessmith

Working on making a new patch

Comment #8

tyjamessmith CreditAttribution: tyjamessmith commented 24 May 2013 at 20:46

Status:

Needs work

» Needs review

File	Size
drupal.remove_template_process_username.1843788.08.patch	1.46 KB

Patch needs review

Comment #9

socketwench CreditAttribution: socketwench commented 24 May 2013 at 23:27

+++ 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.

Comment #10

socketwench CreditAttribution: socketwench commented 24 May 2013 at 23:28

Status:

Needs review

» Needs work

Comment #11

tyjamessmith CreditAttribution: tyjamessmith commented 25 May 2013 at 07:42

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

#8: drupal.remove_template_process_username.1843788.08.patch queued for re-testing.

Comment #12

25 May 2013 at 07:44

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.

Comment #13

scor CreditAttribution: scor commented 28 May 2013 at 19:22

Status:

Needs work

» Needs review

File	Size
1843788_13_template_process_username.patch	5.4 KB

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.

Comment #14

29 May 2013 at 00:25

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

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

Comment #15

tim.plunkett

he/him

English

Philadelphia

CreditAttribution: tim.plunkett commented 29 May 2013 at 00:40

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

#13: 1843788_13_template_process_username.patch queued for re-testing.

Comment #16

scor CreditAttribution: scor commented 29 May 2013 at 01:47

File	Size
1843788_16_template_process_username.patch	5.4 KB

keeping up with HEAD

Comment #17

29 May 2013 at 02:40

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

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

Comment #18

joelpittet

English

Vancouver

CreditAttribution: joelpittet commented 29 May 2013 at 04:01

Status:

Needs work

» Needs review

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

Comment #19

29 May 2013 at 04:21

Status:

Needs review

» Needs work

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

Comment #20

scor CreditAttribution: scor commented 29 May 2013 at 12:52

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)

Comment #21

29 May 2013 at 14:11

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

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

Comment #22

scor CreditAttribution: scor commented 31 May 2013 at 13:35

Status:

Needs work

» Needs review

File	Size
interdiff.txt	1.15 KB
1843788_22_template_process_username.patch	5.63 KB

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

Comment #23

31 May 2013 at 14:38

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

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

Comment #24

scor CreditAttribution: scor commented 31 May 2013 at 17:03

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.

Comment #25

joelpittet

English

Vancouver

CreditAttribution: joelpittet commented 6 June 2013 at 00:29

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

Great work, thanks @scor

Comment #26

star-szr

he/him

English

CreditAttribution: star-szr commented 7 June 2013 at 15:55

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

Needs a reroll.

Comment #27

scor CreditAttribution: scor commented 7 June 2013 at 16:49

Status:

Needs work

» Needs review

File	Size
1843788_27_template_process_username.patch	5.59 KB

no problem.

Comment #28

star-szr

he/him

English

CreditAttribution: star-szr commented 7 June 2013 at 17:23

Status:

Needs review

» Reviewed & tested by the community

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

Comment #29

alexpott

he/they

English

🇪🇺🌍

CreditAttribution: alexpott commented 12 June 2013 at 15:26

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

Comment #30

scor CreditAttribution: scor commented 14 June 2013 at 16:54

Status:

Needs work

» Needs review

File	Size
1843788_30_template_process_username.patch	5.58 KB

Comment #31

scor CreditAttribution: scor commented 14 June 2013 at 19:53

Status:

Needs review

» Reviewed & tested by the community

back to RTBC per #28.

Comment #32

star-szr

he/him

English

CreditAttribution: star-szr commented 14 June 2013 at 20:40

Issue tags:

-Needs reroll

Yup, looks good. Thanks @scor!

Comment #33

alexpott

he/they

English

🇪🇺🌍

CreditAttribution: alexpott commented 14 June 2013 at 22:28

Status:

Reviewed & tested by the community

» Fixed

Committed fa9c197 and pushed to 8.x. Thanks!

Comment #34

Eric_A CreditAttribution: Eric_A commented 15 June 2013 at 08:00

Priority:	Normal	» Major
Status:	Fixed	» Needs review

File	Size
1843788_34_template_process_username.patch	716 bytes

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.