Filter module is not required, but is marked as required

Related: #1548204: Remove user signature and move it to contrib

EnableDisableTest fails, because Filter module cannot be disabled/uninstalled, since User module depends on it.

However, that's an all-optional soft dependency for the user signatures feature only.

Thus, removed that dependency and wrapped the functional code for user signatures into module_exists('filter') conditions.

Same goes for Text module, which depends on Filter module, but is marked as required, for an unknown reason.

Thanks for moving this forward! :)

1. +++ b/core/modules/user/lib/Drupal/user/Tests/UserSignatureTest.php
   @@ -19,7 +19,7 @@ class UserSignatureTest extends WebTestBase {
   -  public static $modules = array('comment');
   +  public static $modules = array('comment', 'filter');
   

   This adjustment appears unnecessary, because comment module depends on text module already?

2. +++ b/core/profiles/standard/standard.info.yml
   @@ -19,6 +19,7 @@ dependencies:
   +  - filter
   
   @@ -29,6 +30,7 @@ dependencies:
   +  - text
   

   Do we need to specify implicit dependencies in installation profiles?

   Hm. Looks like we're doing that already, so this patch just continues to do it.

   I don't really think that specifying implicit dependencies makes sense, as installation profiles should have to care about features only, so ideally we should create an issue to clean that up...

1. +++ b/core/modules/text/text.info.yml
   @@ -6,4 +6,4 @@ version: VERSION
   -required: true
   

   It's great to see that the patch passed.

   However, I'm not sure whether we want to remove the required flag for Text module as part of this issue?

   TBH, I did not think about the consequences of that yet; just Filter module on its own is complex enough already...

   (adding the dependency is definitely within the scope though)

   Perhaps we should defer the removal of the required flag for Text module to a separate issue?

2. +++ b/core/modules/user/lib/Drupal/user/AccountSettingsForm.php
   @@ -151,14 +151,17 @@ public function buildForm(array $form, array &$form_state) {
        // Account settings.
   +    $filter_exists = $this->moduleHandler->moduleExists('filter');
   ...
          '#title' => $this->t('Enable signatures.'),
   -      '#default_value' => $config->get('signatures'),
   +      '#default_value' => $filter_exists ? $config->get('signatures') : 0,
   +      '#access' => $filter_exists,
   

   I wonder whether we need to add a hook_modules_uninstalled() to User module, so as to automatically disable user signatures if Filter module is uninstalled?

   Thoughts?

Restored "required: true" for Text module and enhanced user_modules_uninstalled() to disable user signatures if Filter module is uninstalled.

Over in #1548204: Remove user signature and move it to contrib, that entire conditional code is moved into a new user_signature module, which, by itself, removes User module's dependency on Filter/Text modules (which presents a circular dependency on its own, because Filter depends on User).

That user_signature issue is a very interesting issue and piece of code, because it is a very typical example of a bridge/glue module use-case commonly found in contrib or on custom sites, so I'm (ab)using that issue to some extent to figure out how such modules will architecturally work in D8. — It already has the attention of Entity/Field API maintainers, but I bet you're interested, too; feedback is highly appreciated! :)

That should not prevent this patch from getting in first though — I'm just "offended" by the ugly conditional code and management logic that is required here. ;)

@tstoeckler: The additional condition in AccountSettingsForm is required, because unlike in the user account form, the form element(s) are only hidden via #access (which makes sense, because the "Personalization" container can be extended to contain further personalization settings by contrib modules).

20: drupal8.filter-required.20.patch queued for re-testing.

To my surprise, this patch still applies + passes. :)

Let's move forward here?

As of now, signatures are rendered in template_preprocess_comment(), but the code is wrapped in a condition that checks whether user signatures are enabled. This patch here ensures to disable that setting in case Filter module is uninstalled.

After this patch, I'd like to proceed with #1548204: Remove user signature and move it to contrib, which is going to turn the signature text field into a base field on the User entity + removes all conditional rendering from Comment and User modules.

Adjusted and clarified the code on both aspects.

I think this is ready now.

Note: The change won't have an immediate effect (primarily with respect to test suite performance), since Text module was marked as required for any reason, and Text module depends on Filter module. Resolving that is a separate follow-up issue.

Created that follow-up issue, as mentioned in #18.1 already:

#2191285: Text module is not required, but is marked as required

31: drupal8.filter-required.31.patch queued for re-testing.