Found this issue while working with the module.

  1. Created a content type with an address field.
  2. Set the default county to "Canada"
  3. Created content.
  4. Unset the default county to "None"
  5. 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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chris.smith created an issue. See original summary.

chris.smith’s picture

Title: Default field cannot be reset » Default value for country cannot be reset
andrew.trebble’s picture

I am unable replicate this. I am using the dev version of address and it works correctly for me.

gapple’s picture

Blanca.Esqueda’s picture

Having the same issue!
Set default to -none- is not working, Canada still being the default option.

Blanca.Esqueda’s picture

Using the dev version of address didn't resolve the issue for me.

dww’s picture

Version: 8.x-1.0-beta3 » 8.x-1.x-dev

Confirmed 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:

      $values = $item->isEmpty() ? $this->getInitialValues($country_list) : $item->toArray();

uses the stale value from $item->toArray(); instead of invoking getInitialValues() 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.

dww’s picture

p.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.

Blanca.Esqueda’s picture

Comment 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 use

$item->getEntity()->isNew()

Patch attached, it applies to 8.x-1.0-rc3 and 8.x-1.x-dev.

dww’s picture

Status: Active » Needs review

I 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:

uuid: b1c85762-ccf4-4643-b15f-a3d6b4d33da7
langcode: en
status: true
dependencies:
  config:
    - field.storage.node.field_address
    - node.type.directory_listing
  module:
    - address
id: node.directory_listing.field_address
field_name: field_address
entity_type: node
bundle: directory_listing
label: Address
description: ''
required: false
translatable: false
default_value:
  -
    langcode: null
    country_code: US
    administrative_area: ''
    locality: ''
    dependent_locality: null
    postal_code: ''
    sorting_code: null
    address_line1: ''
    address_line2: ''
    organization: ''
    given_name: ''
    additional_name: null
    family_name: ''
default_value_callback: ''
settings:
  available_countries: {  }
  fields:
    administrativeArea: administrativeArea
    locality: locality
    dependentLocality: dependentLocality
    postalCode: postalCode
    sortingCode: sortingCode
    addressLine1: addressLine1
    addressLine2: addressLine2
    organization: '0'
    givenName: '0'
    additionalName: '0'
    familyName: '0'
  langcode_override: ''
field_type: address

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

Blanca.Esqueda’s picture

Hi,

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

public function isEmpty() {
    $value = $this->country_code;
    return $value === NULL || $value === '';
}

$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:

  1. isEmpty function - overridden for the address field-, is not being used anywhere else in the address module. It seems like it was added only to be used in the condition to set default values or saved values.
  2. As explained in #9, I still think that the right logic is:
    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 (they don't need to be set to default values).

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

dww’s picture

Title: Default value for country cannot be reset » Default country is imposed on fields with an empty address
Status: Needs review » Reviewed & tested by the community

Hi,

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:

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 isEmpty() it makes more sense to use isNew()

(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

dww’s picture

Status: Reviewed & tested by the community » Needs work

Needs reroll now that #2689089: Define a form element type "address" went in.

dww’s picture

Status: Needs work » Needs review
FileSize
942 bytes

Trivial re-roll. I probably shouldn't RTBC my own patch, so leaving this at 'Needs review'.

Blanca.Esqueda’s picture

Status: Needs review » Reviewed & tested by the community

Thanks for the re-roll Derek!

dww’s picture

@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

dww’s picture

Title: Default country is imposed on fields with an empty address » Default country is imposed on fields with an empty address (which currently prevents changing the default country at all)

Making 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.

  • bojanz committed 050f897 on 8.x-1.x authored by Blanca.Esqueda
    Issue #2760387 by dww, Blanca.Esqueda: Default country is imposed on...
bojanz’s picture

Status: Reviewed & tested by the community » Fixed

Committed, thanks!

Let's address the underlying problem in #2838457: Re-enable the default value functionality for Address fields.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

n3uronick’s picture

This 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.

n3uronick’s picture

Here is my quick work-around.

dww’s picture

@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.