Label of contact form message field is not hidden

It seems that it's not an issue with Contact Block module. It should be the issue in Core Contact Module.

I have checked the core contact form and manage display setting for labels and it is not working.

Moving the issue from Contact Module to Drupal core. Fault still exists on 8.3.x.

That's because the message is still hardcoded in the viewbuilder and assumes it is an extra field when it is actually not.

Removing that fixes this.

Also looks like we had zero tests for preview? Which looks laughably bad, but that's another problem.

I wonder why message was considered an extra field. Are there some reasons?

However...

1. +++ b/core/modules/contact/src/Tests/ContactSitewideTest.php
   @@ -339,6 +340,33 @@ function testSiteWideContact() {
   +    $edit = array(
   

   I see below, we're using the square brackets. Let's use also here the square brackets syntax.

2. +++ b/core/modules/contact/src/Tests/ContactSitewideTest.php
   @@ -339,6 +340,33 @@ function testSiteWideContact() {
   +    $this->drupalPostForm('contact/' . $contact_form, $edit, t('Preview'));
   

   $form->toUrl() instead of hardcoding the path, as is the canonical path.

3. +++ b/core/modules/contact/src/Tests/ContactSitewideTest.php
   @@ -339,6 +340,33 @@ function testSiteWideContact() {
   +
      }
   

   Extra empty line before end of method.

More...

+++ b/core/modules/contact/src/Tests/ContactSitewideTest.php
@@ -339,6 +340,33 @@ function testSiteWideContact() {
+    $this->assertText(t($edit['message[0][value]']));
...
+    $this->assertText(t($edit['message[0][value]']));

These are looking wrong. The entity field should not be passed to t().

Also @klausi told me at some point that we should not use t() anymore in tests, not even for buttons, not even in simpletest. I read this somewhere but I can't find the page.

Because message used to be an extra field.

Sorry for those copy paste errors, should be an easy Novice task to fix them.

I'm going to review it

I made a review on 8.1, 2, 3 and 4.x versions and confirm that the patch work.

Thanks for testing, but someone still needs to address #6.

Hi,

As per comment #6 and #7 i have updated the patch.

@arunkumark, please post an interdiff

Re-rolled patch.

@claudiu.cristea can't apply the patch file contact-message-title-2756689-5.patch so the problem in the creation of the interdiff file.

@claudiu.cristea can't apply the patch file contact-message-title-2756689-5.patch so the problem in the creation of the interdiff file.

In such cases, you do first a reroll, commit in a temp branch. Then you apply the changes and run a diff against that intermediary branch, so you get the interdiff.

+++ b/core/modules/contact/src/Tests/ContactSitewideTest.php
@@ -315,11 +316,11 @@ function testSiteWideContact() {
-    $edit = array(
+    $edit = [
...
-    );
+    ];

@@ -335,11 +336,11 @@ function testSiteWideContact() {
-    $edit = array(
+    $edit = [
...
-    );
+    ];

Unrelated changes. Revert them.

And the big problem is that you removed the test from #5.

Corrected patch from #17 As recommended in #18

You lost the test coverage in that reroll. Compare with the previous patches.

@tameeshb, as I said in #18, the patch removed the test from #5. So, please:

• Start a patch from #5,
• Do a reroll
• Apply changes from #6, #7.
• Create and post also an interdiff of those changes, so the reviewers can track what has been changed.

Thank you.

Alright, but the interdiff is to be done with which patch? #5?

@tameeshb, yes, please do it against #5.

Patch form #5 revised, please review! :)

Missed out on a ']' in the last patch, corrected and uploaded with interdiff against #5

@tameeshb, thank you.

1. +++ b/core/modules/contact/src/Tests/ContactSitewideTest.php
   @@ -293,11 +294,11 @@ function testSiteWideContact() {
   -    $edit = array(
   +    $edit = [
   ...
   -    );
   +    ];
   
   @@ -315,11 +316,11 @@ function testSiteWideContact() {
   -    $edit = array(
   +    $edit = [
   ...
   -    );
   +    ];
   
   @@ -335,15 +336,41 @@ function testSiteWideContact() {
   -    $edit = array(
   +    $edit = [
   ...
   -    );
   +    ];
   

   Unrelated changes. Please revert them.

2. +++ b/core/modules/contact/src/Tests/ContactSitewideTest.php
   @@ -335,15 +336,41 @@ function testSiteWideContact() {
   +    $this->drupalPostForm('contact/' . $contact_form, $edit, t('Preview'));
   

   #6.2 is not addressed.

3. +++ b/core/modules/contact/src/Tests/ContactSitewideTest.php
   @@ -335,15 +336,41 @@ function testSiteWideContact() {
   +    $this->drupalPostForm($form->toUrl() . $contact_form, $edit, t('Preview'));
   

   This is wrong. You should not append $contact_form. Only $form->toUrl() should be passed as argument.

Looks good to me. This was originally my patch, but has been RTBC before and reviewed by others too.

Yes, the test is a BrowserTestBase now. I replaced also the deprecated assertions

And because this is a bugfix with no API implications, this should be fixed in 8.3.x.

And back to RTBC, thanks for the reroll and updates.

Less code. More tests. Yay.

Committed and pushed e27d0da to 8.4.x and bbbb980 to 8.3.x. Thanks!

I'm using 8.3.2 and this is still an issue.

Field label is not getting Hidden, Visibly Hidden, nor Inline for me either.

drupal 8.4.3 upgraded from D6
PHP Version 7.1.12
Memory limit 524M
Database Version 5.5.58-0+deb7u1-log

I verified that all the changes in patch #36 are in my 8.4.3 core files.
I tested this on a clean D8 install too but field label is still only Visible.
I also tested if the field is 'not required', but alas, still only visible.

Do you want me to test anything?