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.
Found this issue while working with the module.
- Created a content type with an address field.
- Set the default county to "Canada"
- Created content.
- Unset the default county to "None"
- When I go to create content, the field is still defaulted to "Canada".
Haven't tested in the dev release, but was unable to find a similar issue in the queue.
Comment | File | Size | Author |
---|---|---|---|
#22 | 2760387-21.default-value-country.patch | 996 bytes | n3uronick |
#14 | 2760387-14.default-value-country.patch | 942 bytes | dww |
#9 | address-default_value_country-2760387-9.patch | 823 bytes | Blanca.Esqueda |
Comments
Comment #2
chris.smith CreditAttribution: chris.smith at Portage CyberTech commentedComment #3
andrew.trebble CreditAttribution: andrew.trebble at Portage CyberTech commentedI am unable replicate this. I am using the dev version of address and it works correctly for me.
Comment #4
gappleThis may be related to #2619878: Recoverable fatal error: Argument 1 passed to Drupal\\Core\\Form\\ FormState::setError() must be of the type array, where an error occurs when selecting the 'None'
Comment #5
Blanca.Esqueda CreditAttribution: Blanca.Esqueda as a volunteer and at Portage CyberTech commentedHaving the same issue!
Set default to -none- is not working, Canada still being the default option.
Comment #6
Blanca.Esqueda CreditAttribution: Blanca.Esqueda as a volunteer and at Portage CyberTech commentedUsing the dev version of address didn't resolve the issue for me.
Comment #7
dwwConfirmed that this is broken using -dev (as of commit be940c96).
Initial debugging reveals that AddressDefaultWidget::formElement() is getting invoked with an $items array that has the stale default country. $has_input is FALSE, but so is
$item->isEmpty()
so this line:uses the stale value from
$item->toArray();
instead of invokinggetInitialValues()
to grab the default country as currently configured.Now I just need to understand why formElement() is getting invoked with an $items array with the stale value. ;) Stay tuned.
Comment #8
dwwp.s. The bug is visible whether you had a default country and now the current value is "- None -", or if the new default country is defined but different than the original. So I think #2619878: Recoverable fatal error: Argument 1 passed to Drupal\\Core\\Form\\ FormState::setError() must be of the type array is a red herring, since the bug happens regardless of trying to use "- None -".
p.p.s. The bug survives a 'drush cache-rebuild', so it's not just that some field setting is being cached and not invalidated when the default country is changed.
Comment #9
Blanca.Esqueda CreditAttribution: Blanca.Esqueda as a volunteer and at Portage CyberTech commentedComment on #4 helps to fix another issue (widget ajax crash), but didn't resolve the default value issue.
After some troubleshooting found out that
$item->isEmpty() [line 274]
returns the correct flag when editing content, but it doesn't work properly when adding content (always return Null/False).Default value has to be assigned when adding new content only - as existing content has already saved values even if those are equal to empty.
So, instead of use
$item->isEmpty()
it makes more sense to usePatch attached, it applies to 8.x-1.0-rc3 and 8.x-1.x-dev.
Comment #10
dwwI think Blanca.Esqueda and I nearly cross-posted our attempts at debugging this with #8 + #9. ;) #8 points towards why isEmpty() is returning FALSE. There's a default value already set in the $items array we're passed. Here's the exported config YAML for my field instance:
Notice that the field settings have a default_value defined (using the initial/stale default country from the widget settings), even though the form at admin/structure/types/manage/directory_listing/fields/node.directory_listing.field_address doesn't have a way to define a default value. Ultimately, that's the bug. Somehow that default value is getting set into the field settings, even though Address is trying to hide/disable that.
Anyway, patch #9 does solve the problem. I only wonder if it's a work-around to a deeper problem.
Maybe it's worth ensuring that the field settings never have that default_value defined, since it seems buggy to have a default_value exported into the field configuration that's being ignored like this.
But meanwhile, #9 is definitely an improvement, and anyone who needs this working immediately can safely use it, and I'd be happy to see it committed upstream while we sort out if/how we should address this in other ways.
Thanks!
-Derek
Comment #11
Blanca.Esqueda CreditAttribution: Blanca.Esqueda as a volunteer and at Portage CyberTech commentedHi,
isEmpty() is not returning FALSE because there is a default value assigned.
In fact the real issue is the isEmpty function included in:
/src/Plugin/Field/FieldType/AddressItem.php
$this->country_code
NOT always is the default value.if you go to 'Manage Form Display' and set the default country,
$this->country_code
doesn't get that value right away.$this->country_code
won't be the default value UNTIL the field (Manage Fields) is saved again.Even if the right default value is set in
$this->country_code
, it doesn't make sense to use it to decide if the field is empty.I didn't continue testing on editing content, only adding content so not sure if there is another weird behaviour -- plus I didn't want to spent more time testing when I don't think that the right logic is to use isEmpty() anyway.
Why I don't think that there is a deeper issue:
I was thinking on remove the isEmpty function from /src/Plugin/Field/FieldType/AddressItem.php but I didn't only because a little more testing was needed to be 100% sure that it is not needed in someway.
Regards,
Blanca
Comment #12
dwwHi,
Apologies that I didn't have a chance to get back to this issue sooner. I made time to dig deeper into the bugs I'm talking about and decided to move those to a separate issue. See #2838457: Re-enable the default value functionality for Address fields for all that. I stand by my statement that from the point of view of not being able to change the default country, the patch in #9 is a work-around that masks these other bugs.
However, I'm sorry that I missed this key point from Blanca.Esqueda:
(Originally from comment #9, restated in #11).
I did some testing and verified that's definitely broken -- if you have a default country set, and you edit an entity with an empty address field, the field widget picks the default country instead of leaving it as '- None -' as expected. Patch from #9 definitely solves it.
So, #9 is RTBC for solving *that* bug. As an intended side-effect, it lets people reset their default country. And now that #2838457 is in the queue, I don't mind that we're "hiding" these other bugs with how address fields handle their default_value.
I hope everyone is okay with me splitting those bugs to another issue, and more accurately naming this one.
Thanks!
-Derek
Comment #13
dwwNeeds reroll now that #2689089: Define a form element type "address" went in.
Comment #14
dwwTrivial re-roll. I probably shouldn't RTBC my own patch, so leaving this at 'Needs review'.
Comment #15
Blanca.Esqueda CreditAttribution: Blanca.Esqueda as a volunteer and at Portage CyberTech commentedThanks for the re-roll Derek!
Comment #16
dww@bojanz: It'd be great to commit this before the next release. Ability to change the default country setting is pretty important. I'm tempted to call this a major bug.
Thanks!
-Derek
Comment #17
dwwMaking it a bit more clear why this is an important bug fix to commit ASAP, and to help end-users find this issue if they've run into the problem of not being able to change the default country.
Comment #19
bojanz CreditAttribution: bojanz at Centarro commentedCommitted, thanks!
Let's address the underlying problem in #2838457: Re-enable the default value functionality for Address fields.
Comment #21
n3uronick CreditAttribution: n3uronick commentedThis patch breaks integration with Inline Entity Forms when using Complex widget and creating entities containing address field directly from the parent entity.
To reproduce:
- Add an address field to a content type (A)
- Add an entity reference field (referencing content type above) to another content type (B) and use IEF Complex widget, allowing to create referenced entities
- Create a new node (B) and from there create a new node (A) via IEF Complex widget, populate address field and click on Create button, but not save the parent entity just yet.
- If you will click on Edit, on the created referenced node, addressfield values will not be populated. If after this you will hit Save on the parent entity, then the address information will be lost.
The problem is, that when a new entity is created from a new parent, referenced entity is saved when parent entity is being saved. So $item->getEntity()->isNew() in this edge case will always be true, forcing always using initial values for address, clearing any data entered.
Comment #22
n3uronick CreditAttribution: n3uronick commentedHere is my quick work-around.
Comment #23
dww@n3uronick: The whole way address.module is trying to deal with default values is broken. See #2838457: Re-enable the default value functionality for Address fields for more.