Problem/Motivation

The idea behind the \Drupal\taxonomy_max_depth\Form\FormAltererBase::prepareFormCallback() method is really elegant and clever but the thing is that it creates a Closure which cause fatal errors when Drupal FormBuilder tries to push the form into cache as it tries to serialize it.

Steps to reproduce

  1. Install Drupal
  2. Enable taxonomy max depth and set in on a vocabulary
  3. Add a Media field with Media Library on the same vocabulary
  4. Try to create a term and to add it a media

Expected result: the Media Library opens and you can select one
Current result: the Media Library does not open and you can see a JS error in the console: Exception: Serialization "Closure" is not allowed in serialize()

Note: you can also try with a Paragraph field and try to add a paragraph item on your term.

Proposed resolution

Change the method to return a common callable and serializable array.

Comments

DuaelFr created an issue. See original summary.

duaelfr’s picture

Status: Active » Needs review
StatusFileSize
new2.38 KB

I'm not sure you'll be happy with this patch but it fixes the issue.
Adding the DependencySerializationTrait was necessary to prevent another serialization issue (Database service cannot be serialized and if a deep dependency of EntityTypeManager).

tresti88’s picture

I came across this error after I added, to a vocabulary, an entity reference field that had unlimited cardinality. The Maximum ancestor depth was set to 2. The error appeared whenever i tried to add another entity reference item on the page.

Adding this patch fixed the issue for me.

dmitriy.trt’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

Thank you for a patch! Indeed, this workaround I used before was not reliable, but DependencySerializationTrait seems to be a proper way to handle such cases.

Committed a bit changed version of the patch. Leaving this open mostly for myself, as it would be nice to write a functional test for this specific case before releasing a stable version. But patches with tests are welcome, as always!

  • Dmitriy.trt committed 39adeba on 8.x-1.x
    Issue #3109174 by Dmitriy.trt: Add JS test to make sure AJAX on the term...

  • Dmitriy.trt committed 686f10b on 8.x-1.x
    Issue #3109174 by Dmitriy.trt: Remove max depth from the vocabulary...

  • Dmitriy.trt committed 45700f9 on 8.x-1.x
    Issue #3109174 by Dmitriy.trt: Add core version requirement to the test...
dmitriy.trt’s picture

Status: Needs work » Fixed
Issue tags: -Needs tests

Added JS tests to make sure the issue won't re-appear.

Status: Fixed » Closed (fixed)

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