Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
ContactForm adds a number of fields which are nicely exposed as extra fields. The problem is that a form must explicitly check if a component is enabled before adding it.
Proposed resolution
Wrap name, email and the other defined extra fields in
if ($component = $this->getFormDisplay()->getComponent('name')) {
Remaining tasks
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | test-contact-9.patch | 2.59 KB | claudiu.cristea |
| |||
Comments
Comment #2
larowlanComment #3
T-loI'll work on this (am a novice on core)
Comment #4
T-loI initially understood this bug as "the form display settings aren't working", but they are.
I think you mean that the elements shouldn't be in the form array at all if they are disabled.
If I've understood correctly, this patch should fix the issue.
Comment #5
jibranThis is perfect just needs tests
Comment #6
claudiu.cristeaI manually tested on HEAD. If you hide, for example, the sender's name 'name', in Manage Form Display, then will not be shown on the form. So, it works as expected. Probably somewhere the #access is set based the component visibility.
Here's a test proving that works as designed.
Comment #7
claudiu.cristeaWe don't need these lines.
Comment #8
BerdirYes, access is set in EntityFormDisplay::processForm().
Sorry for the noise, this indeed works. I created this issue based on a ticket we got from a client, who is still on beta9. I thought I had tested it, but apparently it only happens there and only in a special scenario, will need to investigate.
I think I also mixed it up with view extra fields, where it definitely doesn't work and code like this is required.
Not sure if we even want to make the proposed change given that this is indeed supported.
Comment #9
claudiu.cristea@Berdir,
#accessis more elegant and right now is in place. So, it doesn't make sense to add the change. Anyway, this test is proving that this functionality works as expected. I think we can push the test in because I haven't seen a test covering this in Contact module.Thoughts?
Comment #10
claudiu.cristea@Berdir, are we gonna ignore this or we still need this test committed simply as test coverage? I assigned this to you. Feel free to "won't fix" it if case or RTBC if we plan to push it in.
Comment #14
Berdir#2756689: Label of contact form message field is not hidden showed that there actually really was a bug because message is a base field too with widget, but still hardcoded as an extra field in the view builder. Already tested over there now, not sure if we need more tests.