Problem/Motivation

This issue blocks #2173655: Refactor theme() to _theme(); make it a private API to discourage module developers from circumventing the renderable build system. See #2006152: [meta] Don't call theme() directly anywhere outside drupal_render() for instructions.

There are three remaining calls to theme() in Views plugins:

./core/modules/views/lib/Drupal/views/Plugin/views/field/FieldPluginBase.php:    return theme($this->themeFunctions(),
./core/modules/views/lib/Drupal/views/Plugin/views/row/RssFields.php:    return theme($this->themeFunctions(),
./core/modules/views/lib/Drupal/views/Plugin/views/style/Rss.php:    $output = theme($this->themeFunctions(),

Proposed resolution

Review these and see if they can be refactored to not call theme() directly.

Remaining tasks

Git blame these lines to see how and when theme() was reintroduced, then create a patch to convert them. See #2007052: Replace theme() with drupal_render() in views.module for previous conversions in Views.

CommentFileSizeAuthor
#9 2191101-9.patch3.1 KBstar-szr
#9 interdiff.txt615 bytesstar-szr
#7 2191101-7.patch3.09 KBthedavidmeister

Comments

xjm’s picture

xjm
she/her
Primary language English
commented
Related issues: +#2007052: Replace theme() with drupal_render() in views.module
star-szr’s picture

star-szr
he/him
Primary language English
commented
Issue tags: +Twig, +sprint

Thanks @xjm!

thedavidmeister’s picture

thedavidmeister commented
Assigned: Unassigned » thedavidmeister

Having a look...

thedavidmeister’s picture

thedavidmeister commented

FieldPluginBase:

^a626abb handlers/views_handler_field.inc                                           (merlinofchaos       2009-05-17 11:16:51 +0000 1593)   /**
^a626abb handlers/views_handler_field.inc                                           (merlinofchaos       2009-05-17 11:16:51 +0000 1594)    * Call out to the theme() function, which probably just calls render() but
^a626abb handlers/views_handler_field.inc                                           (merlinofchaos       2009-05-17 11:16:51 +0000 1595)    * allows sites to override output fairly easily.
0c8fae0b core/modules/views/lib/Drupal/views/Plugin/views/field/FieldPluginBase.php (Nathaniel Catchpole 2013-10-13 13:15:46 +0100 1596)    *
0c8fae0b core/modules/views/lib/Drupal/views/Plugin/views/field/FieldPluginBase.php (Nathaniel Catchpole 2013-10-13 13:15:46 +0100 1597)    * @param \Drupal\views\ResultRow $values
0c8fae0b core/modules/views/lib/Drupal/views/Plugin/views/field/FieldPluginBase.php (Nathaniel Catchpole 2013-10-13 13:15:46 +0100 1598)    *   Holds single row of a view's result set.
0c8fae0b core/modules/views/lib/Drupal/views/Plugin/views/field/FieldPluginBase.php (Nathaniel Catchpole 2013-10-13 13:15:46 +0100 1599)    *
0c8fae0b core/modules/views/lib/Drupal/views/Plugin/views/field/FieldPluginBase.php (Nathaniel Catchpole 2013-10-13 13:15:46 +0100 1600)    * @return string|false
0c8fae0b core/modules/views/lib/Drupal/views/Plugin/views/field/FieldPluginBase.php (Nathaniel Catchpole 2013-10-13 13:15:46 +0100 1601)    *   Returns rendered output of the given theme implementation.
^a626abb handlers/views_handler_field.inc                                           (merlinofchaos       2009-05-17 11:16:51 +0000 1602)    */
0c8fae0b core/modules/views/lib/Drupal/views/Plugin/views/field/FieldPluginBase.php (Nathaniel Catchpole 2013-10-13 13:15:46 +0100 1603)   function theme(ResultRow $values) {
d4d91db0 lib/Drupal/views/Plugin/views/field/FieldPluginBase.php                    (dereine             2012-08-31 11:20:35 -0400 1604)     return theme($this->themeFunctions(),
^a626abb handlers/views_handler_field.inc                                           (merlinofchaos       2009-05-17 11:16:51 +0000 1605)       array(
^a626abb handlers/views_handler_field.inc                                           (merlinofchaos       2009-05-17 11:16:51 +0000 1606)         'view' => $this->view,
^a626abb handlers/views_handler_field.inc                                           (merlinofchaos       2009-05-17 11:16:51 +0000 1607)         'field' => $this,
^a626abb handlers/views_handler_field.inc                                           (merlinofchaos       2009-05-17 11:16:51 +0000 1608)         'row' => $values
^a626abb handlers/views_handler_field.inc                                           (merlinofchaos       2009-05-17 11:16:51 +0000 1609)       ));
^a626abb handlers/views_handler_field.inc                                           (merlinofchaos       2009-05-17 11:16:51 +0000 1610)   }
thedavidmeister’s picture

thedavidmeister commented

RssFields:

d4d91db0 lib/Drupal/views/Plugin/views/row/RssFields.php (dereine       2012-08-31 11:20:35 -0400 174)     return theme($this->themeFunctions(),
^a626abb plugins/views_plugin_row_rss_fields.inc         (merlinofchaos 2009-05-17 11:16:51 +0000 175)       array(
^a626abb plugins/views_plugin_row_rss_fields.inc         (merlinofchaos 2009-05-17 11:16:51 +0000 176)         'view' => $this->view,
^a626abb plugins/views_plugin_row_rss_fields.inc         (merlinofchaos 2009-05-17 11:16:51 +0000 177)         'options' => $this->options,
^a626abb plugins/views_plugin_row_rss_fields.inc         (merlinofchaos 2009-05-17 11:16:51 +0000 178)         'row' => $item,
^a626abb plugins/views_plugin_row_rss_fields.inc         (merlinofchaos 2009-05-17 11:16:51 +0000 179)         'field_alias' => isset($this->field_alias) ? $this->field_alias : '',
^a626abb plugins/views_plugin_row_rss_fields.inc         (merlinofchaos 2009-05-17 11:16:51 +0000 180)       ));
^a626abb plugins/views_plugin_row_rss_fields.inc         (merlinofchaos 2009-05-17 11:16:51 +0000 181)   }
thedavidmeister’s picture

thedavidmeister commented

Rss.php:

d4d91db0 lib/Drupal/views/Plugin/views/style/Rss.php (dereine       2012-08-31 11:20:35 -0400 137)     $output = theme($this->themeFunctions(),
^a626abb plugins/views_plugin_style_rss.inc          (merlinofchaos 2009-05-17 11:16:51 +0000 138)       array(
^a626abb plugins/views_plugin_style_rss.inc          (merlinofchaos 2009-05-17 11:16:51 +0000 139)         'view' => $this->view,
^a626abb plugins/views_plugin_style_rss.inc          (merlinofchaos 2009-05-17 11:16:51 +0000 140)         'options' => $this->options,
^a626abb plugins/views_plugin_style_rss.inc          (merlinofchaos 2009-05-17 11:16:51 +0000 141)         'rows' => $rows
^a626abb plugins/views_plugin_style_rss.inc          (merlinofchaos 2009-05-17 11:16:51 +0000 142)       ));
^a626abb plugins/views_plugin_style_rss.inc          (merlinofchaos 2009-05-17 11:16:51 +0000 143)     unset($this->view->row_index);
^a626abb plugins/views_plugin_style_rss.inc          (merlinofchaos 2009-05-17 11:16:51 +0000 144)     return $output;
thedavidmeister’s picture

thedavidmeister commented
Assigned: thedavidmeister » Unassigned
StatusFileSize
new 2191101-7.patch3.09 KB

try this.

star-szr’s picture

star-szr
he/him
Primary language English
commented
Status: Active » Needs review

Thanks!

star-szr’s picture

star-szr
he/him
Primary language English
commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new interdiff.txt615 bytes
new 2191101-9.patch3.1 KB

Code looks good and I manually tested all three occurrences, there was no difference in the output. I'm just adding a missing trailing comma here per http://drupal.org/coding-standards#array.

Please cooperate, testbot! :)

star-szr’s picture

star-szr
he/him
Primary language English
commented

Crosslinking with #2111079: Add @code sample and test coverage per hook_theme_suggestions[_HOOK]() and hook_theme_suggestions[_HOOK]_alter() for @thedavidmeister's core review bonus thingy :)

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 0b66890 and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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