Make the class variables protected for FilterFormat

I would like to get this fixed. So I will do a good review for posted patches.

The class variables $format, $name and $weight need to become protected.

I would not rename the class variables. I know there is a @todo is the code. The problem is that renaming a variable will result in a lot of changes. And I have the idea that that will not be allowed (beta changes).

+++ b/core/modules/filter/src/Entity/FilterFormat.php
@@ -428,4 +424,34 @@ protected function calculatePluginDependencies(PluginInspectionInterface $instan
+  /**
+   * {@inheritdoc}
+   */
+  public function getLabel() {
+    return $this->get('label');
+  }
+

This function is not necessary. You can use the build in function label().

+++ b/core/modules/filter/src/Entity/FilterFormat.php
@@ -428,4 +424,34 @@ protected function calculatePluginDependencies(PluginInspectionInterface $instan
+  /**
+   * {@inheritdoc}
+   */
+  public function setLabel($label) {
+    $this->set('label', $label);
+    return $this;
+  }
...
+  /**
+   * {@inheritdoc}
+   */
+  public function setWeight($weight) {
+    $this->set('weight', $weight);
+    return $this;
+  }

These function are probably not used. Maybe only in testing. You can remove them and use set('name', $value) and set('weight', $value)

+++ b/core/modules/filter/filter.module
@@ -246,7 +246,7 @@ function filter_default_format(AccountInterface $account = NULL) {
-  return $format->format;
+  return $format->get('format');

Use the function id() for this instead of get('format').

+++ b/core/modules/filter/src/Tests/FilterAdminTest.php
@@ -261,7 +261,7 @@ function testFilterAdmin() {
+    $this->assertRaw(t('The text format %format has been updated.', array('%format' => $format->get('name'))), 'Full HTML format successfully updated.');

@@ -319,7 +319,7 @@ function testFilterAdmin() {
+    $this->assertRaw(t('The text format %format has been updated.', array('%format' => $format->get('name'))), 'Full HTML format successfully reverted.');

+++ b/core/modules/filter/src/Tests/FilterCrudTest.php
@@ -70,21 +70,21 @@ function testTextFormatCrud() {
+    $t_args = array('%format' => $format->get('name'));
...
+    $this->assertEqual($filter_format->get('name'), $format->get('name'), format_string('filter_format_load: Proper title for text format %format.', $t_args));

Use label() instead of get('name'). In the annotation of the FilterFormat class is the variable $name defined as being the label. And the label has its own getter function called label().

After this change it is RTBC for me. Good work ;-)

All the class variables are protected.
There are already getter functions available, so no need for new ones.
The test-server give it green.
It all looks good to me, so for me it is RTBC.

The file core/modules/text/src/Tests/TextFieldTest.php was updated. So a reroll was necessary. Also for me RTBC.

I thought that the get('name') was replaced by label() in the past. Somehow it got back in. :-(

Double checked it. The file core/modules/text/src/Tests/TextFieldTest.php was updated. So a reroll was necessary. All the get('name') are gone again.

@Wim Leers: How much problems is it giving now?

We need to fix this and add a test to make sure it does not happen again.

Same problem in core/modules/editor/src/Plugin/InPlaceEditor/Editor.php (line 39).

The bug from commend #51 is in file core/modules/editor/src/Form/EditorImageDialog.php (line 51).

Quick fix without new tests.

It is a bit confusing to me when ->format is from the filter_format entity en when it is the property of a text field. So also a patch to only quick fix the bug reported by Win Leers.

My idea was that we do it as a followup in this issue. It seams that that is not allowed, so here is the followup: #2389381: Impossible to add images in WYSIWYG including in-place editing due to fatal error

The issue has been fixed and the followup issue has also been fixed.