Replace theme() with drupal_render() in form.inc [#2190427]

Comment	File	Size	Author
#7	2190427-form-render-element-breaking.patch	701 bytes	joelpittet

#5	2108771-special-attribute-keys-40-reroll.patch	9.11 KB	joelpittet

#3	form-inc-theme-func-2190427-3.patch	1.18 KB	jessebeach

	form-inc-theme-func.patch	1.13 KB	jessebeach

Comment #1

he/him

English

CreditAttribution: star-szr commented 6 February 2014 at 17:26

Thanks!

+++ b/core/includes/form.inc
@@ -2851,7 +2851,14 @@ function theme_form_element_label($variables) {
+    $marker = array(
+      '#theme' => 'form_required_marker',
+      '#element' => $element
+    );

Should have a comma after $element.

Log in or register to post comments

Comment #2

6 February 2014 at 17:34

Status:

Needs review

» Needs work

The last submitted patch, form-inc-theme-func.patch, failed testing.

Log in or register to post comments

Comment #3

jessebeach CreditAttribution: jessebeach commented 6 February 2014 at 17:40

Status:

Needs work

» Needs review

File	Size
form-inc-theme-func-2190427-3.patch	1.18 KB

Comma added.

Log in or register to post comments

Comment #4

joelpittet

English

Vancouver

CreditAttribution: joelpittet commented 6 February 2014 at 17:43

#2152215: Convert theme_form_element_label() to Twig

This is also in the twig conversion. But I'm concerned about that #element => $element there too because of the weird way that 'render element' treats the incoming variables through drupal_render(). In this case form_required_marker doesn't use any incoming variables so it should pass tests no problem. A quick test that that the $variables['element'] has the right things on that required marker. If I get a chance I'll do that, otherwise someone may want to jump on that.

Log in or register to post comments

Comment #5

joelpittet

English

Vancouver

CreditAttribution: joelpittet commented 6 February 2014 at 17:58

File	Size
2108771-special-attribute-keys-40-reroll.patch	9.11 KB

Demo of test breaks and what I mean.

Log in or register to post comments

Comment #6

6 February 2014 at 18:00

Status:

Needs review

» Needs work

The last submitted patch, 5: 2108771-special-attribute-keys-40-reroll.patch, failed testing.

Log in or register to post comments

Comment #7

joelpittet

English

Vancouver

CreditAttribution: joelpittet commented 6 February 2014 at 18:01

Status:

Needs work

» Needs review

File	Size
2190427-form-render-element-breaking.patch	701 bytes

Whoops

Log in or register to post comments

Comment #8

jessebeach CreditAttribution: jessebeach commented 6 February 2014 at 18:02

It's true, the $element passed as #element is not needed for theme_form_required_marker. We could just as easily pass an empty array since the $variables argument to the theme function is never referenced in the theme function.

I would rather deal with that weirdness in #2152215: Convert theme_form_element_label() to Twig, where the mandate is to untangle these inconsistencies and introduce a TWIG template. This issue here is simply concerned with eliminating calls to theme(), which we can safely do with the patch in #3.