Problem/Motivation

After updating entity text field value with leading zeros or plus, the changes are not saved.
The issue was found in Profile entity.

Steps to reproduce:

  1. Add a new "Text (plain)" field to a profile entity on admin/config/people/accounts/fields
  2. Create a new user with the field value "123523" and save
  3. Edit the user and change the value to "0123523"

Expected result:
The field is updated

Actual result:
The field is not updated.

The issue is caused by PHP strings comparison. Currently we check values like $value1 == $value2 which for "123523" == "0123523" will return true.

Proposed resolution

Change comparison to consider types of compared values.
We cannot change comparison to $value1 === $value2. That can lead to incorrect comparison of database values (strings) and current values (can be int).

We need to check strings strictly but save the current comparison of int and string.

Remaining tasks

None.

User interface changes

None.

API changes

None.

Data model changes

None.

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

skrasulevskiy created an issue. See original summary.

skrasulevskiy’s picture

skrasulevskiy’s picture

Status: Active » Needs review

Status: Needs review » Needs work
skrasulevskiy’s picture

skrasulevskiy’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work
skrasulevskiy’s picture

LexBritvin’s picture

Title: FieldItemList::equals() doesn't correctly compare textfield value which is started with zero or plus after updating entity » FieldItemList::equals() doesn't correctly compare textfield values with leading zeros
Version: 8.5.x-dev » 8.4.3
Issue summary: View changes
LexBritvin’s picture

Version: 8.4.3 » 8.5.x-dev
Issue summary: View changes
FileSize
2.38 KB

Wrong patch.

LexBritvin’s picture

Patch from comment #10 is wrong.

Here is updated patch.

Proposed the new comparison mechanism.
Added new test cases.

LexBritvin’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work
LexBritvin’s picture

Very sorry, missed when moved changes.

LexBritvin’s picture

Status: Needs work » Needs review
itsekhmistro’s picture

Hi,

Unfortunately, the patch #14 doesn't fix the original issue ( change of field value "123523" == "0123523").
I have added several tests in FieldItemListTest.php to cover the case and several others that are important for core/tests/Drupal/KernelTests/Core/Entity/ContentEntityChangedTest.php

And modified arrayEquals given in the patch #14.
I still don't like it (arrayEquals) there but currently it does comparison for according to expectations.

Please review.

Status: Needs review » Needs work
itsekhmistro’s picture

Status: Needs review » Needs work

The last submitted patch, 18: field-field-item-list-equals-comparing-2925972-15.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

itsekhmistro’s picture

Status: Needs review » Needs work

The last submitted patch, 20: field-field-item-list-equals-comparing-2925972-15.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

itsekhmistro’s picture

Status: Needs work » Needs review
FileSize
6.11 KB

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.