\Drupal\Core\TypedData\List::setValue() accepts NULL as a value in order to reset the list. However, instead of setting an empty array, it sets NULL as the value. This is done to 'unset' the field, but since it doesn't actually remove the field, this results in a list field having a non-list value. This is bad, because a list without items should be an empty list (array), and not NULL.

CommentFileSizeAuthor
#6 drupal_2082983_6.patch1.47 KBxano
#3 drupal_2082983_3.patch1.4 KBxano
#1 drupal_2082983_1.patch505 bytesxano

Comments

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented
Assigned: xano » Unassigned
Status: Active » Needs review
StatusFileSize
new drupal_2082983_1.patch505 bytes

Status: Needs review » Needs work

The last submitted patch, drupal_2082983_1.patch, failed testing.

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented
Status: Needs work » Needs review
StatusFileSize
new drupal_2082983_3.patch1.4 KB
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Makes sense.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 3: drupal_2082983_3.patch, failed testing.

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented
Status: Needs work » Needs review
StatusFileSize
new drupal_2082983_6.patch1.47 KB

Re-roll.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Assigned: Unassigned » fago

Let's get this confirmed by @fago.

fago’s picture

fago
Primary language German
Location Vienna
commented
Assigned: fago » Unassigned
Status: Needs review » Reviewed & tested by the community

Yeah, I agree that always having a list is preferable. Rest module guy's needed this changed in the past in order to be able to support PATCH, but it looks like this is a thing of the past as tests are still passing.

So, this is good to me and klausi says he's fine as well if his tests are passing.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Nice catch!

Committed and pushed to 8.x. Thanks!

  • Commit 901ae98 on 8.x by webchick: 
    Issue #2082983 by Xano: \Drupal\Core\TypedData\List accepts non-list...

Status: Fixed » Closed (fixed)

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