When you disable a text format it is not possible to re-enable it. This seems counter to the behavior in other situations. For instance, in Views, it's possible to Disable a display and Enable it later.

And here's how Views works:

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Bug because of unexpected behaviour. Upon disabling the text format it disappears from the form which makes it seem deleted, but it isn't actually deleted because you can't add a new text format with the same machine name.
Issue priority Major because as cilefen states in #19:

Actually, there is no workaround for GUI users (imagine "I want my text format back!"), so this is major.

Prioritized changes The main goal of this issue is two-fold, fixing a DrupalWTF bug and improving usability.
Disruption Non-disruptive because only the filter format admin interface is changed. No API changes
Files: 

Comments

vegantriathlete’s picture

Issue summary: View changes
cilefen’s picture

Title: Disable / Enable Text Formats » Disabling a text format actually removes it. This is confusing.
Issue tags: +Usability
cilefen’s picture

Version: 8.1.x-dev » 8.0.x-dev
cilefen’s picture

Category: Feature request » Bug report
shumer’s picture

FileSize
2.02 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 98,259 pass(es), 17 fail(s), and 0 exception(s). View

This patch change label and text for more reliable "Delete" instead "Disable".

shumer’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 5: 2502637-5.patch, failed testing.

shumer’s picture

Status: Needs work » Needs review
FileSize
5.71 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 98,278 pass(es). View

Errors fixed.

legolasbo’s picture

Status: Needs review » Needs work
  1. +++ b/core/modules/filter/src/FilterFormatListBuilder.php
    @@ -126,6 +126,11 @@ public function getDefaultOperations(EntityInterface $entity) {
    +    // Change operation label to Delete which more reliable for user.
    +    if ($entity->hasLinkTemplate('disable')) {
    +      $operations['disable']['title'] = t('Delete');
    +    }
    +
    

    Isn't there a link template 'delete' which can be used?

  2. +++ b/core/modules/filter/src/Tests/FilterAdminTest.php
    @@ -265,7 +265,7 @@ function testFilterAdmin() {
    -    $this->drupalPostForm('admin/config/content/formats/manage/' . $format->id() . '/disable', array(), t('Disable'));
    +    $this->drupalPostForm('admin/config/content/formats/manage/' . $format->id() . '/disable', array(), t('Delete'));
    
    +++ b/core/modules/filter/src/Tests/FilterHooksTest.php
    @@ -69,7 +69,7 @@ function testFilterHooks() {
    -    $this->drupalPostForm('admin/config/content/formats/manage/' . $format_id . '/disable', array(), t('Disable'));
    +    $this->drupalPostForm('admin/config/content/formats/manage/' . $format_id . '/disable', array(), t('Delete'));
    
    +++ b/core/modules/filter/src/Tests/FilterSecurityTest.php
    @@ -75,7 +75,7 @@ function testDisableFilterModule() {
    -    $this->drupalPostForm('admin/config/content/formats/manage/' . $format_id . '/disable', array(), t('Disable'));
    +    $this->drupalPostForm('admin/config/content/formats/manage/' . $format_id . '/disable', array(), t('Delete'));
    
    +++ b/core/modules/text/src/Tests/TextFieldTest.php
    @@ -116,7 +116,7 @@ function _testTextfieldWidgetsFormatted($field_type, $widget_type) {
    -        $this->drupalPostForm('admin/config/content/formats/manage/' . $format->id() . '/disable', array(), t('Disable'));
    +        $this->drupalPostForm('admin/config/content/formats/manage/' . $format->id() . '/disable', array(), t('Delete'));
    

    The path should also be /delete

vegantriathlete’s picture

It's NOT actually deleting the text format. I don't think it's a good call to rename it Delete. That would imply that the text format could be created again, which is not the case. If you try to create the text format again you will get an error along the lines that it already exists.

If the text is going to be change to Delete, then the format should actually be deleted.

If the text is going to stay Disable, then the ability to re-enable it should be added.

legolasbo’s picture

@vegantriathlete,

It's NOT actually deleting the text format.

I only reviewed the code, not the functionality even though i guessed as much. Which is why i hinted at using an actual delete link.

cilefen’s picture

Title: Disabling a text format actually removes it. This is confusing. » Disabled text format can't be seen in the GUI

Is that an accurate title?

cilefen’s picture

Title: Disabled text format can't be seen in the GUI » Disabled text formats can't be seen in the GUI
shumer’s picture

Status: Needs work » Needs review
FileSize
1.7 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 98,491 pass(es). View

Ok, let's try one more time )
Now disabled format will appear in the formats list, with warning message.

legolasbo’s picture

Status: Needs review » Needs work
FileSize
86.19 KB
legolasbo’s picture

The screenshot below shows the confirmation page. It's text is no longer correct.
Incorrect text.

The screenshot below shows the "Text formats and editors" page after disabling a text format. There is no way for the user to re-enable a text format, nor is it possible to create a new format with the same name. We should add an action button to enable the text-format once it has been disabled.
No action buttons.

  1. +++ b/core/modules/filter/src/FilterFormatListBuilder.php
    @@ -111,6 +101,12 @@ public function buildRow(EntityInterface $entity) {
    +      $roles_markup = SafeMarkup::placeholder($this->t('This format is disabled and no more available.
    

    This format has been disabled and is no longer available.

  2. +++ b/core/modules/filter/src/FilterFormatListBuilder.php
    @@ -111,6 +101,12 @@ public function buildRow(EntityInterface $entity) {
    +        Any content stored with that format will not be displayed.'));
    

    Any content stored with this format will not be displayed.

shumer’s picture

Status: Needs work » Needs review
FileSize
7.48 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 98,579 pass(es). View

Enable action added.

cilefen’s picture

#17 seems to work in manual testing. I am tagging "Needs usability review" so a specialist can chime in.

cilefen’s picture

Priority: Normal » Major

Actually, there is no workaround for GUI users (imagine "I want my text format back!"), so this is major.

shumer’s picture

I hope it will pass review )

cilefen’s picture

The status quo is the same behavior in Drupal 7. But, why disallow the re-enabling of a text format?

legolasbo’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests
+++ b/core/modules/filter/src/Entity/FilterFormat.php
@@ -194,6 +196,21 @@ public function disable() {
+    // Allow modules to react on text format deletion.

enabling?

+++ b/core/modules/filter/src/Form/FilterEnableForm.php
@@ -0,0 +1,57 @@
+    return $this->t('You are going to enable text format.');

That seems obvious to me, I think this line should explain the result of that action. i.e. Any content stored with this format will be displayed again.

Also, this needs tests

shumer’s picture

Status: Needs work » Needs review
FileSize
9.02 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 98,576 pass(es), 1 fail(s), and 0 exception(s). View

Text fixed and test for format enable operation added.

Status: Needs review » Needs work

The last submitted patch, 23: 2502637-23.patch, failed testing.

shumer’s picture

Status: Needs work » Needs review
FileSize
7.88 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 98,577 pass(es). View

Looks like writing tests it's not my best feature. Just adding patch with fixed text.

legolasbo’s picture

@shumer,

Reviewing your changes would be a lot easier for me if you would provide an iterdiff. As for the tests, I can probably take a look at those tomorrow.

shumer’s picture

FileSize
1.52 KB

@legolasbo, interdiff attached

legolasbo’s picture

Issue tags: -Needs tests
FileSize
9.9 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 98,613 pass(es). View
4.22 KB

Attached patch adds tests and some small textual changes.

cilefen’s picture

Issue tags: +Needs beta evaluation

This needs a beta evaluation or it should be pushed back to 8.1.x. I am thinking we should push it back because there is so much going on right now and HEAD is the same as the Drupal 7 behavior.

legolasbo’s picture

Issue summary: View changes

Added beta phase evaluation

legolasbo’s picture

Issue tags: -Needs beta evaluation
legolasbo’s picture

Issue summary: View changes
Mark LaCroix’s picture

I thought I'd chime in here to say that while I appreciate the impulse to push a solution for this to 8.1.x, it brought up another problem for a simple single-user site I'm building which feels like a showstopper:

The warning message when disabling a text format says "any content stored with that format will not be displayed." However, this is wrong (at least in my case), as content using a format I disabled is still live on the site (localhost), even after clearing the cache. However, what's odd is I am now prevented from editing any nodes that had used that format.

When I try to edit one of these nodes, I get the dreaded "The website encountered an unexpected error. Please try again later." message. I can just delete the nodes, of course, and a user shouldn't be too surprised to lose field content if they delete a text format, but for many users struck by this, content in other fields in affected nodes might be very important (assuming this issue is not just limited to me, which is possible).

The warning message says "this cannot be undone," which I assumed meant was that I was actually deleting the format, and could recreate it using the same machine name, but as pointed out, this is impossible, and leaves cruft in the database.

The attempt to explain what is actually happening in the warning text is a strange thing because what it actually does is reinforce Drupal's notorious reputation for obtuse helper text, so even if it were accurate, it's easy to ignore it's intended meaning.

At the very least, it should be an "expected" error, not an "unexpected" one, and if it's not just me, indicates that this is a bigger issue than already recognized.

legolasbo’s picture

@Mark LaCroix,

We can still address this in 8.0.x as per the beta phase evaluation. If you could review the code that would be another step in the right direction.

Can you provide steps to reproduce the unexpected error? If you can, please open a new issue for it and reference it here. What you are describing sounds like something that's major or even critical.

Mark LaCroix’s picture

Done and done! Thanks for the clue:

https://www.drupal.org/node/2555973

SGhosh’s picture

FileSize
9.82 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 106,885 pass(es). View

Patch updated wrt latest code in 8.0.x branch.

pjonckiere’s picture

+++ b/core/modules/filter/src/FilterFormatListBuilder.php
@@ -71,16 +71,6 @@ public function getFormId() {
-  public function load() {
-    // Only list enabled filters.
-    return array_filter(parent::load(), function ($entity) {
-      return $entity->status();
-    });
-  }

This looks like a design choice. What would have been the reason?

legolasbo’s picture

@pjonckiere,

That code was added in #1987124: Convert filter_admin_format_page() and filter_admin_overview() to a Controller, when the callbacks were converted to a controller. The issue we are currently working on was actually discovered by dawehner while reviewing that patch. That method was added to keep the functionality the same as in D7. This behaviour is really strange however and should also be fixed in D7 in my opinion, which warrants it's removal.

yoroy’s picture

Adding two related issues from the D7 era that may shed some light on why things currently work as they do. This is a complex topic :-)

swentel’s picture

Xano’s picture

FileSize
9.79 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 108,210 pass(es). View

Re-roll.

Xano’s picture

FileSize
1.35 KB
9.39 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 108,174 pass(es), 25 fail(s), and 0 exception(s). View

The enable and disable routes need CSRF tokens, because they respond to GET requests, but change the site's state.

+++ b/core/modules/filter/src/FilterFormatListBuilder.php
@@ -134,6 +129,16 @@ public function getDefaultOperations(EntityInterface $entity) {
+    // Remove Disable and Configure operation for already disabled formats.
+    if (!$entity->status()) {
+      if (isset($operations['disable'])) {
+        unset($operations['disable']);
+      }
+      if (isset($operations['edit'])) {
+        unset($operations['edit']);
+      }
+    }

Why was this added? We do not prevent any entities of other types from being edited when they are disabled, and the parent method already hides the disable operation for already disabled formats.

Status: Needs review » Needs work

The last submitted patch, 42: drupal_2502637_42.patch, failed testing.

legolasbo’s picture

Assigned: Unassigned » legolasbo

Patch no longer appies, rerolling and fixing tests.

legolasbo’s picture

Assigned: legolasbo » Unassigned
Status: Needs work » Needs review
FileSize
3.04 KB
10.02 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 110,405 pass(es), 11 fail(s), and 0 exception(s). View

Rerolled and fixed tests. Also made a small interface text change.

legolasbo’s picture

Attributed organisation.

Status: Needs review » Needs work

The last submitted patch, 45: disabled_text_formats-2502637-45.patch, failed testing.

legolasbo’s picture

Status: Needs work » Needs review
FileSize
5.29 KB
5.29 KB

Also fixed the remaining tests.

legolasbo’s picture

FileSize
43.07 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 110,462 pass(es). View

Oops

Bojhan’s picture

Issue tags: -Needs usability review

Nothing to review, sounds like a straight bug.

cilefen’s picture

FileSize
10 KB

I think #45 is the latest patch and this is the reroll.

Status: Needs review » Needs work

The last submitted patch, 51: disabled_text_formats-2502637-51.patch, failed testing.

cilefen’s picture

Status: Needs work » Needs review
FileSize
1014 bytes
11 KB

I am ignorant of how to handle CSRF in WebTests. Does anyone know how that is worked-around?

Status: Needs review » Needs work

The last submitted patch, 53: disabled_text_formats-2502637-53.patch, failed testing.

cilefen’s picture

Status: Needs work » Needs review
FileSize
799 bytes
10.96 KB

I was overthinking it.

cilefen’s picture

The last submitted patch, 55: disabled_text_formats-2502637-55.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 56: disabled_text_formats-2502637-56.patch, failed testing.

cilefen’s picture

Status: Needs work » Needs review
FileSize
1.42 KB
13.28 KB

Tests should pass now.

Pepe Roni’s picture

It is possible to reenable disabled text formats with the configuration manager:

  • go to /admin/config/development/configuration/single/export
  • choose Text format as the Configuration type
  • choose the disabled text format as Configuration Name
  • copy the configuration
  • go to /admin/config/development/configuration/single/import
  • again choose Text format as the Configuration type
  • paste the configuration
  • change "status" from false to true
  • click on import

Voilá, the text format is available again. This is working but not user friendly :(

PS. It may be that I have not used the correct English labels; I'm currently working with German translations.

cilefen’s picture

Issue tags: +needs backport to D7

We should backport this.

diqidoq’s picture

TBH, I am not sure if I've tracked that issue correctly because it looks like it has been gone off course a little bit. To come back to the OP text:

When you disable a text format it is not possible to re-enable it.

Exactly. This is an obvious UI logic caveat against common UX standarts regarding the common understanding of visually "disabling" vs. "deactivating or deleting" something.

Since (like reported) the format is indeed NOT deleted and still there but disabled, with unxpected side effects like being hidden, disabling content which is using it, being unable to enable at the same place etc. We can talk about multiple issues here:

  1. UI/UX issue: It should be greyed out, but not hidden, as long as the format exists in db.
  2. it should be poss. to re-enable it on the same place: this is the UI-logic of "disabling" something.
  3. It should not disable the content which uses it already, unless it is wanted (in a second step or other option)

Respecting all issues and side effects not obvious for all from the first look the selection dropdown should include:

  1. delete (delete the format completely from db but keep the content with a warning and a fallback format for this content, like we did with deleted users in D7 with the content keeping option). Why? because how else would we finally delete a format then if not here? #UX
  2. disable/turn off (disable the format and its content and make the format no more selectable for editors)
  3. inactive (disable the format but keep its content with format fallback and make the format no more selectable for editors)

And only the delete option should make this text format disappear from the list of text formats if we want to stay in common practice of UI concepts and UX.

cilefen’s picture

Version: 8.0.x-dev » 8.1.x-dev

@diqidoq I agree with most of what you wrote. We should do one of the following:

  • Change the issue to include everything in #62.
  • Use this issue as a "quick win", change the UI a little but keep the current core functionality. Open a follow-up to do #62.
caten8’s picture

Thanks to Pepe Roni, #60 works.

By the way, if you have any text in those fields and they disappeared after disabling your text formats, don't panic. Just run update.php after re-enabling them and your stored content will return.

Also, if you'd rather make changes to the database, under D8, text format configurations are now in the "config" table.
Run this SQL Query:
SELECT `collection`, name FROM `config` WHERE name LIKE 'filter.format%';
and you'll find all four default text formats listed.
Your data is stored in a BLOB field. You can download it to examine what's inside. If you're comfortable with db queries you can run an UPDATE to change status from 0 to 1 but be careful with SET commands, they're powerful and can overwrite other data unless you know what you're doing. You can also expose BLOB contents by changing settings in your phpMyAdmin config file. Here's a helpful post to get you started:
https://groups.drupal.org/node/134269

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

tkoleary’s picture

Version: 8.2.x-dev » 8.3.x-dev
Issue tags: +sprint
tkoleary’s picture

Status: Needs review » Needs work

Needs a new patch to reflect the comments in #62

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.