If you try to save the tab options form, you will have no joy.

The validation falls through to displayPluginBase::validateOptionsForm when it assumes any section with _options appended, such as style_options, tab_options is a plugin type. This is true for style but not for tab. So it then goes on to try and find a plugin manager for 'plugin.manager.views.tab' which does not exist.

We need to stop this from happening, so we can either check this is a plugin type, or change the logic a bit more. I think the first is fine. Also needs a UI test for the tab options...

Files: 
CommentFileSizeAuthor
#21 2012502-21.patch6.63 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 57,643 pass(es).
[ View ]
#21 interdiff-2012502-21.txt739 bytesdamiankloip
#17 2012502-17.patch6.63 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 55,623 pass(es).
[ View ]
#17 interdiff-2012502-17.txt3.01 KBdamiankloip
#12 2012502-12-test-only.patch1.82 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 55,422 pass(es), 2 fail(s), and 0 exception(s).
[ View ]
#12 2012502-12.patch3.01 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 55,429 pass(es).
[ View ]
#12 interdiff-2012502-12.txt1.96 KBdamiankloip
#7 2012502-7-tests-only.patch1.81 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 56,871 pass(es), 3 fail(s), and 0 exception(s).
[ View ]
#7 2012502-7.patch2.59 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 56,771 pass(es), 2 fail(s), and 0 exception(s).
[ View ]
#6 2012502-6.patch2.59 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#6 interdiff-2012502-6.txt1.89 KBdamiankloip
#3 2012502-3-test-only.patch1.74 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 56,460 pass(es).
[ View ]
#3 2012502-3.patch2.5 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 57,134 pass(es).
[ View ]
#3 interdiff-2012502-3.txt990 bytesdamiankloip
#1 2012502-test-only.patch1.63 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 56,015 pass(es), 2 fail(s), and 0 exception(s).
[ View ]
#1 2012502-with-fix.patch2.39 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 55,037 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new2.39 KB
FAILED: [[SimpleTest]]: [MySQL] 55,037 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
new1.63 KB
FAILED: [[SimpleTest]]: [MySQL] 56,015 pass(es), 2 fail(s), and 0 exception(s).
[ View ]

Status:Needs review» Needs work

The last submitted patch, 2012502-with-fix.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new990 bytes
new2.5 KB
PASSED: [[SimpleTest]]: [MySQL] 57,134 pass(es).
[ View ]
new1.74 KB
PASSED: [[SimpleTest]]: [MySQL] 56,460 pass(es).
[ View ]

Waaaa, Let's try that again.

Status:Needs review» Needs work
Issue tags:-VDC

The last submitted patch, 2012502-3.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+VDC

#3: 2012502-3.patch queued for re-testing.

StatusFileSize
new1.89 KB
new2.59 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

Let's actually expose some failures ;)

StatusFileSize
new2.59 KB
FAILED: [[SimpleTest]]: [MySQL] 56,771 pass(es), 2 fail(s), and 0 exception(s).
[ View ]
new1.81 KB
FAILED: [[SimpleTest]]: [MySQL] 56,871 pass(es), 3 fail(s), and 0 exception(s).
[ View ]

Sorry, just tests and tests with fix. interdiff from #6 is still correct.

Status:Needs review» Needs work
Issue tags:-VDC

The last submitted patch, 2012502-7.patch, failed testing.

Status:Needs work» Needs review

#7: 2012502-7.patch queued for re-testing.

#7: 2012502-7-tests-only.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+VDC

The last submitted patch, 2012502-7.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.96 KB
new3.01 KB
PASSED: [[SimpleTest]]: [MySQL] 55,429 pass(es).
[ View ]
new1.82 KB
FAILED: [[SimpleTest]]: [MySQL] 55,422 pass(es), 2 fail(s), and 0 exception(s).
[ View ]

OK, Pull yourself together. This is really not a hard patch...

Status:Needs review» Needs work
Issue tags:-VDC

The last submitted patch, 2012502-12.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+VDC

#12: 2012502-12.patch queued for re-testing.

Priority:Normal» Major

I think this is major.

index 07423cb..b6f56af 100644
--- a/core/modules/views/lib/Drupal/views/Plugin/views/display/DisplayPluginBase.php
--- a/core/modules/views/lib/Drupal/views/Plugin/views/display/DisplayPluginBase.php
+++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/DisplayPluginBase.phpundefined
+++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/DisplayPluginBase.phpundefined
@@ -811,13 +811,20 @@ public function usesFields() {
@@ -811,13 +811,20 @@ public function usesFields() {
   public function getPlugin($type) {
     // Look up the plugin name to use for this instance.
     $options = $this->getOption($type);
-    $name = $options['type'];
+
+    // Return now if no options have been loaded.
+    if (empty($options) || !isset($options['type'])) {
+      return;
+    }
     // Query plugins allow specifying a specific query class per base table.
     if ($type == 'query') {
       $views_data = Views::viewsData()->get($this->view->storage->get('base_table'));
       $name = isset($views_data['table']['base']['query_id']) ? $views_data['table']['base']['query_id'] : 'views_query';
     }
+    else {
+      $name = $options['type'];

Some dedicated test here could help.

+++ b/core/modules/views_ui/lib/Drupal/views_ui/Tests/DisplayPath.phpundefined
@@ -55,4 +55,31 @@ public function testDeleteWithNoPath() {
+    $this->drupalPost("admin/structure/views/nojs/display/test_view/page_1/path", array('path' => $this->randomString()), t('Apply'));

Nitpick ... this should use single quotes ...

StatusFileSize
new3.01 KB
new6.63 KB
PASSED: [[SimpleTest]]: [MySQL] 55,623 pass(es).
[ View ]

Good idea, Let's add some tests for getPlugin(). This should cover the functionality.

Status:Needs review» Reviewed & tested by the community

Great! I assume it will be green.

Status:Reviewed & tested by the community» Needs work

+++ b/core/modules/views/lib/Drupal/views/Tests/Plugin/DisplayUnitTest.phpundefined
@@ -87,7 +93,35 @@ public function testDefaultOptions() {
+    // Get a plugin twice, and make sure the same instance is returned.
+    $view->destroy();
+    $view->initDisplay();
+    $first = spl_object_hash($display_handler->getPlugin('style'));
+    $second = spl_object_hash($display_handler->getPlugin('style'));
+    $this->assertIdentical($first, $second, 'A cached plugin instance was returned.');

The assertion message here does not seem correct...

@alexpott, what do you think this should say? 'A statically cached plugin instance was returned', 'The same plugin instance was returned'?

Assigned:Unassigned» alexpott
Status:Needs work» Needs review
StatusFileSize
new739 bytes
new6.63 KB
PASSED: [[SimpleTest]]: [MySQL] 57,643 pass(es).
[ View ]

@alexpott, rerolled with that comment tweak.

Status:Needs review» Reviewed & tested by the community

The new assertion message looks fine.

Status:Reviewed & tested by the community» Fixed

Committed 4b815b3 and pushed to 8.x. Thanks!

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