Files: 
CommentFileSizeAuthor
#2 2007052-2.patch9.97 KBthedavidmeister
PASSED: [[SimpleTest]]: [MySQL] 56,331 pass(es).
[ View ]
#2 2007052-interdiff-1-2.txt1.01 KBthedavidmeister
#1 2007052-1.patch10.07 KBthedavidmeister
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/views/lib/Drupal/views/Plugin/views/display/DisplayPluginBase.php.
[ View ]

Comments

thedavidmeister’s picture

Status:Active» Needs review
StatusFileSize
new10.07 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/views/lib/Drupal/views/Plugin/views/display/DisplayPluginBase.php.
[ View ]

patch.

thedavidmeister’s picture

StatusFileSize
new1.01 KB
new9.97 KB
PASSED: [[SimpleTest]]: [MySQL] 56,331 pass(es).
[ View ]

fix for fatal error.

Status:Needs review» Needs work

The last submitted patch, 2007052-2.patch, failed testing.

c4rl’s picture

Title:convert theme() to drupal_render() in views module» Replace theme() with drupal_render() in views.module

Retitling

thedavidmeister’s picture

Status:Needs work» Needs review

#2: 2007052-2.patch queued for re-testing.

thedavidmeister’s picture

Cottser’s picture

Status:Needs review» Reviewed & tested by the community
Issue tags:-Novice, -Needs manual testing

I've reviewed this patch a couple times now, looks great to me. I think careful reviews will be more beneficial than manual testing for these patches and had a quick chat with @thedavidmeister about this on IRC just now. I will be updating the issue summary on the meta issue slightly to make the scope of these conversions a bit smaller.

Cottser’s picture

Status:Reviewed & tested by the community» Needs review
Issue tags:+Needs manual testing
+++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/DisplayPluginBase.phpundefined
@@ -1848,7 +1849,10 @@ public function buildOptionsForm(&$form, &$form_state) {
-          '#markup' => '<div class="form-item">' . theme('item_list', array('items' => $funcs)) . '</div>',
+          '#theme' => 'item_list',
+          '#prefix' => '<div class="form-item">',
+          '#items' => $funcs,
+          '#suffix' => '</div>',

Okay discussed on IRC, at least this bit could use some manual testing :)

Miroling’s picture

Issue tags:+CodeSprintUA

Taked for manual testing #2009672

Miroling’s picture

Status:Needs review» Reviewed & tested by the community

Behaviour in views ui didn't change. Patch working fine.

catch’s picture

Status:Reviewed & tested by the community» Fixed

Committed/pushed to 8.x, thanks!

alexpott’s picture

Status:Fixed» Needs work
+++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/DisplayPluginBase.phpundefined
@@ -2083,8 +2087,11 @@ protected function formatThemes($themes) {
-    return theme('item_list', array('items' => array_reverse($fixed)));
+    $item_list = array(
+      '#theme' => 'item_list',
+      '#items' => array_reverse($fixed),
+    );
+    return drupal_render($item_list);

let's not do unnecessary assignments... just do

  return(drupal_render(array(
    '#theme' => 'item_list',
    '#items' => array_reverse($fixed),
  )));

Therefore...

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/field/PrerenderList.phpundefined
@@ -79,12 +79,13 @@ function render_items($items) {
-        return theme('item_list',
-          array(
-            'items' => $items,
-            'title' => NULL,
-            'type' => $this->options['type']
-          ));
+        $item_list = array(
+          '#theme' => 'item_list',
+          '#items' => $items,
+          '#title' => NULL,
+          '#type' => $this->options['type'],
+        );
+        return drupal_render($item_list);

Same here...

+++ b/core/modules/views/views.theme.incundefined
@@ -1181,14 +1185,16 @@ function theme_views_mini_pager($vars) {
-  return theme('item_list',
-    array(
-      'items' => $items,
-      'title' => NULL,
-      'type' => 'ul',
-      'attributes' => array('class' => array('pager')),
-    )
+  $item_list = array(
+    '#theme' => 'item_list',
+    '#items' => $items,
+    '#title' => NULL,
+    '#type' => 'ul',
+    '#attributes' => array(
+      'class' => array('pager'),
+    ),
   );
+  return drupal_render($item_list);

Same here...

alexpott’s picture

Status:Needs work» Fixed

Awesome cross post... back to fixed then...

tim.plunkett’s picture

drupal_render() takes the element by reference anyway, we needed those assignments.
Awesome cross post indeed.

thedavidmeister’s picture

If you don't do the assignment first you get a fatal error.

I really thought I had left a note about this on the meta issue summary but apparently I didn't, sorry.

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