Overview of issue:
Disable the ckeditor from a vanilla install of drupal 8, then try to add a new piece of article content you will get the following error message:

"Error message
Twig_Error_Runtime: An exception has been thrown during the rendering of a template ("The plugin (ckeditor) did not specify an instance class.") in "core/themes/seven/templates/page.html.twig" at line 103. in Twig_Template->displayWithErrorHandling() (line 279 of /home/quickstart/websites/aiyps.com/core/vendor/twig/twig/lib/Twig/Template.php).
The website has encountered an error. Please try again later. "

Expected behaviour
Was not expecting the error message, was expecting to be able to create article content.

Replicate
Install drupal 8
Goto Menu > Extend
de-select CKeditor
click "save configuration"

goto Menu > Content > Add Content
Select Article

You will receive error message.

Drupal 8 package details
Drupal 8 packaged version: 8.0-alpha1+186-dev
Last updated: May 29, 2013 - 12:45

Files: 
CommentFileSizeAuthor
#22 editor_uninstall_labels_missing-2007248-22.patch756 bytesWim Leers
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 67,707 pass(es).
[ View ]
#20 Screen Shot 2014-03-21 at 13.33.46.png102.71 KBWim Leers
#3 editor-2007248.pass_.patch1.86 KBlarowlan
PASSED: [[SimpleTest]]: [MySQL] 57,924 pass(es).
[ View ]

Comments

I can confirm this error happens in the current build in git.

A bit more sleuthing and I figured out the editor gets set in and loaded from the following config files:

editor.editor.basic_html.yml
editor.editor.full_html.yml

Remove or rename those files and clear cache after disabling CKEditor and everything operates as expected.

Issue tags:+Needs tests

Tagging

Title:TWIG Error Runtime - when disabling ckeditor from modules listWhen ckeditor is disabled, editor config entities that refer to it cause a Twig Exception
Status:Active» Needs review
StatusFileSize
new1.86 KB
PASSED: [[SimpleTest]]: [MySQL] 57,924 pass(es).
[ View ]
new1.3 KB
FAILED: [[SimpleTest]]: [MySQL] 57,102 pass(es), 50 fail(s), and 11 exception(s).
[ View ]

Fail and pass

Component:theme system» editor.module

Correct module

Status:Needs review» Needs work

This patch deletes the configuration data of CKEditor upon disabling the module. This isn't standard practice in Drupal (yet, see #1199946: Disabled modules are broken beyond repair so the "disable" functionality needs to be removed), where the expected behavior is that the user can turn back on a disabled module and the configuration will be restored as it was before the module was disabled.

Instead of deleting all configuration as part of hook_disable(), the typical approach to this problem is to check if the requirements are met before attempting to load the editor. When an editor entity is attempted to be loaded, we should check that the corresponding editor plugin still exists. If it doesn't, we should return FALSE, as if the editor did not exist at all.

Status:Needs work» Needs review
Issue tags:-Needs tests
StatusFileSize
new3.74 KB
PASSED: [[SimpleTest]]: [MySQL] 56,061 pass(es).
[ View ]
new3.01 KB

So this version doesn't delete the entities, it just catches the PluginException in the storage controller and doesn't load the editor entity. Lets see what this breaks.

Green!

StatusFileSize
new1.87 KB
new3.42 KB
PASSED: [[SimpleTest]]: [MySQL] 57,991 pass(es).
[ View ]

I'd try to minimize the overridden part by calling the parent when possible.

Title:When ckeditor is disabled, editor config entities that refer to it cause a Twig ExceptionWhen CKEditor is disabled, editor config entities that refer to it cause a Twig Exception
Issue tags:+CKEditor in core

Great catch :) Will try to review later today. I dislike that we introduce a custom storage controller, but I guess that's the only way?

#8: ckeditor-2007248-8.patch queued for re-testing.

Title:When CKEditor is disabled, editor config entities that refer to it cause a Twig ExceptionWhen CKEditor is disabled, editor config entities that refer to it cause a Twig exception
Status:Needs review» Needs work
Issue tags:+sprint, +Spark

Isn't this really just user error? It's the same as disabling a module that provides a new filter without removing that filter from all text formats where it was in use. That will also break.
So: are we really solving the right problem here?


In any case, this patch doesn't cover all use cases, the Twig exception still occurs when accessing admin/config/content/formats (using otherwise the same scenario as in the issue summary).

AFAICT the reason is editor_load(), which calls entity_load_multiple('editor') (i.e. loads all 'editor' entities), which in turn causes the override to have no effect, because it'll fall back to the parent implementation:

    if ($ids === NULL) {
      $result = parent::buildQuery($ids, $revision_id);
    }

Issue tags:-sprint

.

@WimLeers well my preference is to just plain delete it, as per patch at #3 but @quicksketch asked for something less aggressive.

Status:Needs work» Postponed

#13: Good point.

#5 no longer holds, since consensus has been reached at #1199946: Disabled modules are broken beyond repair so the "disable" functionality needs to be removed: the "disabled" module state will be removed.

Which means the patch in #3 is good to go, except that it should use hook_uninstall() rather than hook_disable(), because the latter is being removed in #1199946: Disabled modules are broken beyond repair so the "disable" functionality needs to be removed. Postponing on that issue getting committed, then we should reroll the patch in #3.

Status:Postponed» Needs work

The method in #3 relies on every implementing module (CKEditor in this case, but imagine TinyMCE as another) dealing with this on disable and/or uninstall (note that for disable, I would think disabling the plugin a la https://drupal.org/node/1926376 may be a better way to go). So shouldn't it be dealt with at least one level up (in the Editor module) if not higher?

And there are other ways besides disable/uninstall that a plugin can disappear. For example, a module which defined several editor plugins and then removed one (think WYSIWYG module dropping support for a particular editor) would have to do the same thing on update...

And in the general case, is the approach in #3 even possible? Does the implementing module always know all the places that use it?

So overall seems like this issue might be a duplicate of #1881630: [meta] Determine how to respond to invalid plugins (plugin dependencies)...

Status:Needs work» Closed (duplicate)

Thanks for your thoughtful feedback, David_Rothstein — much appreciated!

I agree with your overall assessment. So: marking this issue as a duplicate of #1881630: [meta] Determine how to respond to invalid plugins (plugin dependencies).

Issue summary:View changes
Status:Closed (duplicate)» Postponed

I think this should be postponed on the meta, not necessarily closed until such time as there is a resolution there.
Thoughts?

I guess you're right.

I assumed this was going to be fixed by #1881630: [meta] Determine how to respond to invalid plugins (plugin dependencies) but you're implying that we'll still need to do some work here to leverage the work done in #1881630 (once it lands) — which is probably right.

Talked this through with alexpott on IRC. #2080823: Create API to discover content or config entities' soft dependencies and use this to present a confirm form on module uninstall is getting into shape which allows us to set dependencies in config entities and are automatically discovered on uninstall (and also deleted). However, to keep the size and complexity of the other issue relatively small, we'll follow up on this when that one is committed and decide then whether to remove this on hook_uninstall() or use the soft dependency api.

Title:When CKEditor is disabled, editor config entities that refer to it cause a Twig exceptionWhen CKEditor module is uninstalled, the right text editor config entities are affected but their labels are missing
StatusFileSize
new102.71 KB

#2080823: Create API to discover content or config entities' soft dependencies and use this to present a confirm form on module uninstall got committed. That fixed this problem! :)

However, in testing it, I found a minor bug: text editor entity labels are missing.

So let's repurpose this issue to fix that.

Priority:Normal» Minor
Status:Postponed» Active

Status:Active» Needs review
Issue tags:+sprint, +Novice
StatusFileSize
new756 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 67,707 pass(es).
[ View ]

The cause was simple: Text Editor config entities themselves don't have labels; their associated Filter Format config entity does!