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...
Comment | File | Size | Author |
---|---|---|---|
#21 | 2012502-21.patch | 6.63 KB | damiankloip |
#21 | interdiff-2012502-21.txt | 739 bytes | damiankloip |
#17 | 2012502-17.patch | 6.63 KB | damiankloip |
#17 | interdiff-2012502-17.txt | 3.01 KB | damiankloip |
#12 | 2012502-12-test-only.patch | 1.82 KB | damiankloip |
Comments
Comment #1
damiankloip CreditAttribution: damiankloip commentedComment #3
damiankloip CreditAttribution: damiankloip commentedWaaaa, Let's try that again.
Comment #5
damiankloip CreditAttribution: damiankloip commented#3: 2012502-3.patch queued for re-testing.
Comment #6
damiankloip CreditAttribution: damiankloip commentedLet's actually expose some failures ;)
Comment #7
damiankloip CreditAttribution: damiankloip commentedSorry, just tests and tests with fix. interdiff from #6 is still correct.
Comment #9
damiankloip CreditAttribution: damiankloip commented#7: 2012502-7.patch queued for re-testing.
Comment #10
damiankloip CreditAttribution: damiankloip commented#7: 2012502-7-tests-only.patch queued for re-testing.
Comment #12
damiankloip CreditAttribution: damiankloip commentedOK, Pull yourself together. This is really not a hard patch...
Comment #14
damiankloip CreditAttribution: damiankloip commented#12: 2012502-12.patch queued for re-testing.
Comment #15
damiankloip CreditAttribution: damiankloip commentedI think this is major.
Comment #16
dawehnerSome dedicated test here could help.
Nitpick ... this should use single quotes ...
Comment #17
damiankloip CreditAttribution: damiankloip commentedGood idea, Let's add some tests for getPlugin(). This should cover the functionality.
Comment #18
dawehnerGreat! I assume it will be green.
Comment #19
alexpottThe assertion message here does not seem correct...
Comment #20
damiankloip CreditAttribution: damiankloip commented@alexpott, what do you think this should say? 'A statically cached plugin instance was returned', 'The same plugin instance was returned'?
Comment #21
damiankloip CreditAttribution: damiankloip commented@alexpott, rerolled with that comment tweak.
Comment #22
tim.plunkettThe new assertion message looks fine.
Comment #23
alexpottCommitted 4b815b3 and pushed to 8.x. Thanks!