Problem/Motivation

When settings the default value for a "Text (formatted)" field (either as a textfield, textarea, or textarea with summary), if you only set the default format, the result is not saved. In order to save the default format, you must enter non-whitespace text in the default value field. This is a usability issue as site builders want to specify a given text format (and, thus, the WYSIWYG editor config or the lack of an editor altogether) for a field without entering default text.

Proposed resolution

Include the format value in \Drupal\text\Plugin\Field\FieldType\TextItemBase::isEmpty().

Remaining tasks

  1. Write patch.
  2. Write tests.
  3. Write change record for TextItemBase::isEmpty

User interface changes

None. Other than the expected result of having your settings saved! :)

API changes

Classes that extend TextItemBase will need to consider the change to the isEmpty() method.

Data model changes

None.

Original report

I try to set a default text format to a field (Long text) but it's not saved if i don't set a default value for the field.

1. I edit the field
2. I set the Text processing to "Filtered text (user selects text format)"
3. I set "Text format" to a another text format
4. Then i save

If i also set a default value for my field, my text format is set as default
But if i leave the default value of my field, it's not saved.

The function "text_field_is_empty (field/modules/text/text.module - line 51-)" is called. This function check only the value for the field, not the format. It return true and the function "_field_filter_items (field/field.module - line 51-)" unset this item including my text format.

If i change the function function text_field_is_empty

function text_field_is_empty($item, $field) {
  if ((!isset($item['value']) || $item['value'] === '') && (!isset($item['format']) || $item['format'] === '')) {
    return !isset($item['summary']) || $item['summary'] === '';
  }
  return FALSE;
}

to

function text_field_is_empty($item, $field) {
  if (!isset($item['value']) || $item['value'] === '')) {
    return !isset($item['summary']) || $item['summary'] === '';
  }
  return FALSE;
}

my text format is set as default event i don't enter a default value for my field.

But i'm not sure it's the good way to fix this issue.

CommentFileSizeAuthor
#59 1278886-51-59-interiff.txt5.89 KBgnuget
#59 1278886-59-default-text-formats.patch6.53 KBgnuget
#51 1278886-51-default-text-formats.patch659 bytesmikeker
#39 allow_default_text_format_per_field-1278886-39.patch1.71 KBjenlampton
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] Unable to apply patch allow_default_text_format_per_field-1278886-39.patch. Unable to apply patch. See the log in the details link for more information. View
#39 allow_default_text_format_per_field-1278886-39-do-not-test.patch1.66 KBjenlampton
#17 allow_default_text_format_per_field-1278886-17.patch1.71 KBKevin Morse
PASSED: [[SimpleTest]]: [MySQL] 36,678 pass(es). View
#13 allow_default_text_format_per_field-1278886-13.patch1022 bytesjenlampton
FAILED: [[SimpleTest]]: [MySQL] 36,671 pass(es), 1 fail(s), and 0 exception(s). View
#13 allow_default_text_format_per_field-1278886-13-do-not-test.patch1002 bytesjenlampton
#8 allow_default_text_format_per_field-1278886-8.patch684 bytesjenlampton
PASSED: [[SimpleTest]]: [MySQL] 34,596 pass(es). View
#8 allow_default_text_format_per_field-1278886-8-D7.patch664 bytesjenlampton
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch allow_default_text_format_per_field-1278886-8-D7.patch. Unable to apply patch. See the log in the details link for more information. View
#7 allow_default_text_format_per_field-1278886-7.patch684 bytesjenlampton
PASSED: [[SimpleTest]]: [MySQL] 34,608 pass(es). View
#7 allow_break_tag-881006-64-D7.patch1.78 KBjenlampton
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch allow_break_tag-881006-64-D7_0.patch. Unable to apply patch. See the log in the details link for more information. View
#3 save-default-text-format-1278886-3.patch655 bytesbrenk28
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch save-default-text-format-1278886-3_0.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots. View
#1 save-default-text-format-1278886-1.patch664 bytesRaphael Apard
PASSED: [[SimpleTest]]: [MySQL] 35,976 pass(es). View
Members fund testing for the Drupal project. Drupal Association Learn more

Comments

Raphael Apard’s picture

Version: 7.8 » 7.x-dev
FileSize
664 bytes
PASSED: [[SimpleTest]]: [MySQL] 35,976 pass(es). View

I test this with dev version of drupal, no module and i have the same result : can't set a default text format for a text field if Default Value is null.
I made a patch (works for me) but maybe there is a better way to fix this.

marcingy’s picture

Status: Active » Needs review

Setting as needs review for the bot

brenk28’s picture

FileSize
655 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch save-default-text-format-1278886-3_0.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots. View

Here is a version to use in a make file

Status: Needs review » Needs work

The last submitted patch, save-default-text-format-1278886-3.patch, failed testing.

Raphael Apard’s picture

The only difference between patch #1 and #3 is the prefix leading slashes. I think patch #3 use -p0.

marcingy’s picture

Version: 7.x-dev » 8.x-dev

This needs to be fixed in d8 first

jenlampton’s picture

Status: Needs work » Needs review
Issue tags: +Novice
FileSize
1.78 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch allow_break_tag-881006-64-D7_0.patch. Unable to apply patch. See the log in the details link for more information. View
684 bytes
PASSED: [[SimpleTest]]: [MySQL] 34,608 pass(es). View

Here's a patch against D8, ready for review. And a new D7 version for after that one gets in :)

jenlampton’s picture

FileSize
664 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch allow_default_text_format_per_field-1278886-8-D7.patch. Unable to apply patch. See the log in the details link for more information. View
684 bytes
PASSED: [[SimpleTest]]: [MySQL] 34,596 pass(es). View

hm, lets try those again.

jenlampton’s picture

Status: Needs work » Needs review

Comment from @hefox:

the patch is changing the logic to say having a format other than blank format means the field is not empty even if there's nada in it, which makes me suspect might cause issues assuming hook_is_empty is used elsewhere.

I added that in the case of content, we don't want the content saved if there's only a text-format (but no content) provided, but in the case of *default values* we do. It seems we should be saving differently for default values than for content.

scottalan’s picture

Title: Set a default format text to a text long field not saved » Default text formats are not saved properly without accompanying values

+ if ((!isset($item['value']) || $item['value']) === '' && (!isset($item['format']) || $item['format'] === '')) {

Should this have been:

+ if ((!isset($item['value']) || $item['value'] === '') && (!isset($item['format']) || $item['format'] === '')) {

Just asking for my own clarity :)

Status: Needs review » Needs work

The last submitted patch, allow_default_text_format_per_field-1278886-8-D7.patch, failed testing.

jenlampton’s picture

Status: Needs review » Needs work

@scottalan yes, good catch with the parenthesis!

jenlampton’s picture

Status: Needs work » Needs review
FileSize
1002 bytes
1022 bytes
FAILED: [[SimpleTest]]: [MySQL] 36,671 pass(es), 1 fail(s), and 0 exception(s). View

Okay, here's a new approach that just avoids the empty check if the field is being used to set a default value (i.e. where $entity is empty)

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

The last submitted patch, allow_default_text_format_per_field-1278886-13.patch, failed testing.

Kevin Morse’s picture

Status: Needs work » Needs review

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

The last submitted patch, allow_default_text_format_per_field-1278886-13.patch, failed testing.

Kevin Morse’s picture

Status: Needs work » Needs review
FileSize
1.71 KB
PASSED: [[SimpleTest]]: [MySQL] 36,678 pass(es). View

The patch in 13 worked for me.

I have attached a patch which cleans up some white space and should also include a small fix to the test that was failing which should make it pass. I've never modified a test before so hopefully this modification is correct.

cweagans’s picture

GiorgosK’s picture

#12 d7 patch works for latest drupal 7.18

David Hernández’s picture

Hi there,

I'm not sure if this change is really necessary. Do we really need to save the input format of the default value if the default value is empty?

If the default value is empty, means that the user will have to write the content of the field and then, the input format selected by default should be on "Text Processing".

I'm not sure if I'm not understanding correctly the issue or the issue itself doesn't make any sense.

By the way the solution for tests failing (#17) is not changing the test, but fixing the issue that is causing the test to fail. A test should only be changed if the requirement has changed too.

I think this works as designed and the issue can be closed.

Regards,
David.

sun’s picture

Component: field system » filter.module
Status: Needs review » Postponed (maintainer needs more info)

Reading the issue summary, I tend to agree with #20.

The only possible way this could result in an actual problem is if you'd have a filter enabled for a particular format, which inserts (arbitrary) content. Only in that case, an empty text value would be a valid and an actual value, since it is dynamically enhanced upon view. Thus, if the format does not get saved, the automatic enhancement does not happen, because the selected format was not stored.

However, I'm not aware of any filter implementation that turns "" (nothing) into something concrete (and it sounds a bit silly).

Therefore, I'd agree with won't fix.

That is, unless there is a more concrete issue summary that clarifies what exactly breaks without a fix for this and why this is needed.

Kevin Morse’s picture

Status: Postponed (maintainer needs more info) » Needs review
Issue tags: +Needs usability review

I encountered this issue when I inherited a site with multiple text formats (Filtered HTML, Full HTML, and WYSIWYG HTML) all of which had slightly different validation rules and one of which used CKEditor.

The site was primarily used by non tech savvy users who would constantly complain that they couldn't figure out how to change the font or insert links into articles they were writing. The reason being the "Filtered HTML" format would always be selected when they clicked Add content. I believe this is because it is by default weighted higher on the Content authoring / Text formats page. "Text Processing" only allows the user to choose between "Plain text" and "Filtered text" it does not allow a default "Text format" to be specified.

The only way to have anything other than highest weighted format appear when a user clicks Add content is to enter text into the Default text box. I just entered, "Write your article here" in the Default value and then changed to the WYSIWYG text format.

While this is not a technical problem it is definitely a usability problem. If a user wants to specify a default Text format (other than the highest weighted format) they need to be able to do so. Right now the only apparent option way to do so is by choosing a Text format in the Default Value field. Unfortunately this doesn't work.

mitron’s picture

Category: bug » feature

There is a consensus that the current functionality is the intended functionality, this it is not a bug.

Kevin Morse’s picture

Category: feature » bug

I don't believe that two or three people is consensus, especially when a big focus of D8 is usability. If I change anything inside of a frame and then press save on the frame, what I changed should be saved.

When I encountered this, I searched the Issues for what I thought was a bug and found there was already a bug report created for it. If anything you could say there is just as much consensus that this is a bug. Although we have confirmed that Drupal is indeed doing what it was designed to do, I believe this is a bug on the grounds of usability. That is why I have tagged this with Needs usability review.

Until someone from the usability team looks into this please leave it in its original category of Bug Report. I do agree with sun that a fix should be postponed while evidence is gathered.

Thanks,

Kevin

mitron’s picture

@Kevin Morse: Can you give a link for the other bug report on this?

sun’s picture

Status: Needs review » Postponed (maintainer needs more info)

re: #22:

Thanks for clarifying. The point where I lost you is "default text format".

Filter module provides a pretty advanced concept for default text formats already. It automatically figures out which formats are available and intersects that with the formats that the current user is able to access. Out of that limited set of formats, the first one (weighing lowest) is the current user's default format.

That part seems to be missing in your equation, as your use-case seems to be to create a new, empty node, with a pre-configured text format? And you're making the bold assumption that the "other" user who's supposed to edit that node (also) has access to the text format you selected?

If the current user does not have access to the selected text format, then all the user sees is a disabled textarea that informs him/her that its content cannot be edited due to access constraints.

Paulmicha’s picture

@see #1109010: Did not save custom text format configuration in body fields and #1015510: Specify a default format for text fields (not long text)

I also find it should be considered a serious usability issue - steps for my case :

1. Create several input formats (e.g. 'full_html', and 'simple_html'), both accessible to the user / role who's testing (or admin)
NB: I used 'full_html' as default input format for body fields

2. Create a custom field, long text, with the text format 'simple_html'

3. When you edit the node with this custom field, the default selected input format is 'full_html' (and thus, the appearance of the Wysiwyg which can have a different button setup per input format, is the wrong one)

Current workaround (which I'm gonna test now that I know it's a core issue) : @see http://drupal.org/node/1015510#comment-5099898

function MODULENAME_form_CONTENTTYPE_node_form_alter(&$form, &$form_state) {
  $form['field_example']['und'][0]['#format'] = "filtered_html";

  return $form;
}
sun’s picture

The use-case that you're describing rather sounds like #784672: Allow text field to enforce a specific text format

yoroy’s picture

Tagging

trainingcity’s picture

This is definitely a problem. In a social media site where non-technical users need to be defaulted to different text formats based on issues such as the type of content form they are completing, the current design is useless, forcing everyone to receive the highest weighed text format associated with their user type. In a site where all new users are authenticated users, this means they have only one default text format, and must know to change it on form entry, something that cannot be expected of non-technical users.

The work around is to enter default text in the form field. This is not a viable solution.

dpolant’s picture

If you're using features, there is a slightly better workaround:

1) Enter a default value in the field where you want to set the filter
2) Set the filter type to what you want
3) Save
4) Update your feature with this current state
5) Edit the feature and replace the default value with an empty string

Now your feature will provide the UX that you want.

capellic’s picture

1. Install the Better Features module.
2. Go here (admin/config/content/formats/settings) and check the "Use field default" checkbox
3. Go back to your field and save the default text format again
4. Profit.

jan.ptacek’s picture

1. should read Install the Better Formats module.

unfortunately it is marked as unstable for D7
falling back to: #27
... 3 years old bug ... it was such an error to deploy a site on drupal :(

manningpete’s picture

Issue tags: -Novice

Removing the novice tag.

Why: because issue is postponed, contentious and/or lacking consensus.

Novice tag documentation: https://www.drupal.org/core-mentoring/novice-tasks

Jelle_S’s picture

Status: Postponed (maintainer needs more info) » Active

I'm not sure what more info can be provided here, so setting back to Active.

I consider myself to be a quite advanced Drupal user. Yet I was baffled when I saved field instance settings, setting a "default" text format, then to edit it again and see that this setting was not saved.

In my view it's quite a simple problem:
There is a form, which I edit, so I expect that when I reload the form, everyting I entered is still the same.
But it's not, which confuses the user, hence it's a usability issue.

I think the explantions in #22 and #24 are spot-on.

klidifia’s picture

Might be best to so something like this then. I wanted one format for the initial node and another format for all commenting:

function MODULE_form_alter(&$form, &$form_state, $form_id) {
  switch ($form_id) {
    case 'custom_page_node_form':
      $form['body'][LANGUAGE_NONE][0]['#format'] = "format_one";
      break;

    case 'comment_node_custom_page_form':
      $form['comment_body'][LANGUAGE_NONE][0]['#format'] = "format_two";
      break;

    default:
  }
}
jenlampton’s picture

I'm still using this patch on all my sites. In my view this is definitely a UX problem for the site-builder, and therefore a core bug. The solution also offers a UX improvement to the user entering content. win-win!

@Sun's comment in #26 is a valid one. Drupal does have a complicated process for figuring out what the "default" text format should be based on the current user and their level of access.

However, the user + access calculation will result in the same "default" format everywhere on a Drupal site, and in reality, not all filtered text fields are equal. You may have one field where you want users to have a WYSIWYG (for example, body) and others where you specifically do not want users to have a wysiwyg. Drupal's calculations do not take this into effect.

Though it's possible to demand that everyone who encounters that problem should provide either a custom module with a `form_alter` in it, or require that they use a complicated set of contrib modules, the solution in this patch is much simpler.

@Sun, what's the harm in setting a default format? A user who does not have access to the default won't get it, they'll get the behavior we see now. But a user who does have access to that format gets a UX improvement. I'm not sure I understand the hesitation here.

jenlampton’s picture

Status: Active » Needs review

changing status.

jenlampton’s picture

FileSize
1.66 KB
1.71 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] Unable to apply patch allow_default_text_format_per_field-1278886-39.patch. Unable to apply patch. See the log in the details link for more information. View

Also, I updated the Drupal 7 patch to match fixes in #17. Attaching them both again here.

Status: Needs review » Needs work

The last submitted patch, 39: allow_default_text_format_per_field-1278886-39.patch, failed testing.

akosipax’s picture

I applied the patch in #39 to my D7 site. The default is applied when adding a node but not when editing a node. The text format defaults to the first listed.

jenlampton’s picture

@akosipax once a node is saved it will have a format associated with the text. Defaults will only show before the node is saved, otherwise it will pull whatever was in the database for each, and default to that.

akosipax’s picture

Right. Just to be clear, let's say I have the following text_formats in the following order:

filtered_html
full_html

  1. I configured body's default to be full_html.
  2. I create a node. body's default format is full_html as expected.
  3. I save the node without touching the body field; That is, body is empty and has the default full_html.
  4. I edit the node that I just created; body is empty and has filtered_html as the selected text format.

In the last step, I was expecting the format to be full_html. Can you reproduce this as well? If so, is this intentional behaviour?

akosipax’s picture

Also, I'm having the following behaviour with the patch. I don't think this is intentional since I imagine it is a generally unwanted behaviour. You can try the following steps with a core field type like "Text"

1) Access settings of a field (either a new field or an existing field; /admin/structure/types/manage/[node_type]/fields/[field_name])
2) Save the settings of the field without entering default value for that field.
3) Create a new node [that has that field whose settings we just saved]
4) Save the node without entering anything in that field.

Result: The "empty" data of that field is saved. field_data_[field_name] has a row for that empty / NULL field.
Expected: That field should not be considered empty and should not be saved. field_data_[field_name] should not have any row for that empty / NULL field.

Additional Notes:
The empty data is cleared if you edit the node and save it again -- without entering anything in the field.
I believe this has to do with the changes in the patch:

 function field_default_submit($entity_type, $entity, $field, $instance, $langcode, &$items, $form, &$form_state) {
-  // Filter out empty values.
-  $items = _field_filter_items($field, $items);
+  // Empty values are okay for setting default values so we want to avoid
+  // calling _field_filter_items(). Example: set a default text-format.
+  if (!empty($entity)){
+    // Filter out empty values.
+    $items = _field_filter_items($field, $items);
+  }
   // Reorder items to account for drag-n-drop reordering.
   $items = _field_sort_items($field, $items);

If I undo that change, it behaves normally again.

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

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

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

The last submitted patch, 39: allow_default_text_format_per_field-1278886-39.patch, failed testing.

The last submitted patch, 39: allow_default_text_format_per_field-1278886-39.patch, failed testing.

amklose’s picture

Any update on this for 8.1.x? I was going to try making a new patch but I can't even find this file or function in 8.1.1

colan’s picture

I'm wondering if things could change here after #784672: Allow text field to enforce a specific text format gets in. If so, we should postpone this one until after that one's done.

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.

mikeker’s picture

OK, hoping to breath some new life into this 5+ year old issue...

Attached is a patch that fixes the problem that just changing the default text format for a formatted text field does not get saved. Needs tests which I hope to get to soon.

No interdiff as code seems to have changed substantially since #39.

mikeker’s picture

Issue summary: View changes
Issue tags: +Needs change record

If we change the functionality of TextItemBase::isEmpty, we'll need a CR. Added an issue summary.

Status: Needs review » Needs work

The last submitted patch, 51: 1278886-51-default-text-formats.patch, failed testing.

BartK’s picture

Here's a quick workaround for setting Full HTML as default in Drupal 8:

1. Select "Full HTML" in the text format box.
2. Click the "source" button in the editor.
3. Enter " " (without the quotes) in the text box and DO NOT CLICK THE SOURCE BUTTON AGAIN OR THE EDITOR WILL REMOVE IT.
4. Save your changes to the field.

Note that the editor strips the   tag every time, so you'll have to do this every time you make changes to the field. On the other hand, the   tag won't show up when the user creates new content because the editor trims it automatically when the page is loaded, so that's good at least.

Grienauer’s picture

1278886-51-default-text-formats.patch works from my point of view… tested it and saves default formatter as expected :)
thx!

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.

slanger’s picture

Status: Needs work » Reviewed & tested by the community

FYI: Patch #51 worked for me, too (although in Drupal 8.2.3). Thanks, @mikeker!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 51: 1278886-51-default-text-formats.patch, failed testing.

gnuget’s picture

Hi!

I had this problem in one of my recent projects so I want to help to finally fix it.

I fixed all the migrate tests, now that isEmpty takes into account the filter the migrate tests need the "filter" module so it can use the filter plugins when it migrates the data.

There is still 1 test failing I will work on that and in the tests this week)

Thanks for all your work so far.

gnuget’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 59: 1278886-59-default-text-formats.patch, failed testing.

gnuget’s picture

After to check the last failing test I think we are using the wrong approach.

Having this:

 return ($value === NULL || $value === '') && ($format === NULL || $format === '');

I think this will make to isEmpty()return false the 99% of the times because the fields by default have a format, that is why all the migrate tests now required the filter module because now it had to deal with the filter because isEmpty() is returning false...

I will start fresh and work on a different approach.

HongPong’s picture

IMHO it would be better to break the return line down into a few lines instead of one line with that many comparison operators. Could help debugging later.

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

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

joseph.olstad’s picture

gnuget’s picture

Ideally somehow we would need to compare the previous textformat with the new one if the field is empty and the textformat is the same the isEmpty() should return true... but no idea how access the previous value from there. :-/

The workaround on #54 does not work if the field doesn't have the Editor. :-(

Version: 8.5.x-dev » 8.6.x-dev

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