Uncaught TypeError: this.$pluginSelect.find(...)[0] is undefined

Same problem

Hi, This issue occurs while upgrading the version from d9 to d9.

Could you please provide steps to reproduce the issue with screenshots?

Thanks,

This seems to be a problem with extra fields. Steps to reproduce:

1. Install Drupal 9.4.x with the standard installation profile.
2. Login as admin
3. Navigate to Admin / Structure / Contact Forms
4. Choose Manage Form Display from the operations drop-down on the Website Feedback contact form
5. Move extra field Send copy to sender to the Disabled section
6. Move extra field Send copy to sender back to the into the section with active controls
7. Observer the error on the browser console

I guess that #3239134: Refactor (if feasible) uses of the jQuery val function to use VanillaJS introduced this problem. this.$pluginSelect.find('option')[0] doesn't seem return anything for extra fields. The most elegant solution would be to use optional chaining operator here.

This is still a problem in 9.5.x. Posting a patch for that branch, needs to be backported afterwards I guess.

Patch is failing to apply and it will need a test if it's a bug.

Indeed. The patch is just an attempt to point to the problematic line. It would be awesome if somebody who knows the frontend workflow could pickup here.

Also I'm unsure whether it is okay to use operator chaining, since this seems to be a recent addition to JavaScript IIRC. Would be cool if we get feedback from frontend folks in this regard.

So I tried to replicate actually.

On D9.5
Claro admin theme.

Went to admin/structure/types/manage/article/display
Dragged fields around 20+ times
Dragged fields around 10+ times, saved, dragged around another 20+ times.

Can't replicate the issue.

Moving to PNMI since it's a new issue but tagging for needs steps to reproduce.

This affects extra fields (not normal fields), please try the repro in #8 (contacts has lots of extra fields). The issue is reproducible 100% in D9.4 and D9.5. I strongly suspect that #3239134: Refactor (if feasible) uses of the jQuery val function to use VanillaJS caused the regression from D9.3.

If you like to repro this using content types instead of contact forms, then do the following:

1. Navigate to admin/structure/types/manage/article/display
2. Drag the Links extra field from the Disabled section into the section with active controls

Yup following the new steps I was able to replicate.

Moving to 10.1 because not sure how else to test for console errors.

And https://www.drupal.org/project/drupal/issues/3293090 made it into 10.x

javascript console error fixed. please check and review.

@SandeepSingh199 thank you for the patch but it failed to apply. And is missing the tests. For contributions please read the existing tags and comments for what is needed and test the patch locally. Removing credit as #19 should be ignored.

This makes a core feature of Drupal nearly unusable for a sitebuilder who doesn't know how to turn off JavaScript (selecting "show weights" is only a workaround if done before and instead of trying to drag; the dragging stays stuck). Upping to major.

Reroll of #9 for Drupal 10.1. JavaScript ES6 build step was removed in #3278415: Remove usages of the JavaScript ES6 build step, the build step itself, and associated dev dependencies, hence we can work on the source file directly. Also support for Internet Explorer was dropped a while ago, thus optional chaining can be used in core JS now.

Still needs tests. Switching to needs review nevertheless in order to trigger a test-run.

I guess, tests should go here core/modules/field_ui/tests/src/FunctionalJavascript/EntityDisplayTest.php, I'm going to try cooking up something.

Ok, so it seems we cannot use optional changing right now because scripts/dev/commit-code-check.sh is not ready for that yet.

Thus, reroll of #19 with line wrapping adapted in a way to make the patch a bit easier to read. Still needs tests.

Simple test case added to the existing testExtraFields() method.

Uhm, the test-only patch passes even though it should fail. Setting to needs work, I hope that somebody who has a better understanding of the frontend test suite will pick this up.

Stepping through the tests with php debugger and chromium inspector side-by-side revealed that minks dragTo() method doesn't really work exactly like a manual drag-drop action. E.g., a manual drag-drop will invoke Drupal.fieldUIOverview.onSwap() repeatedly until the mouse button is released. Minks dragTo() results in no invocation of onSwap() at all.

In order to trigger the error it is necessary to run the test case in a separate test method and drag-drop #display-extra-field-hidden (extra field which is initially in hidden area) on top of #display-extra-field (extra field which is initially in content area).

Regrettably #30 did not fail the expected test.

Filed #3331835: Some JavaScript errors are not recorded during test runs.

So, with the patch in #3331835: Some JavaScript errors are not recorded during test runs and the test-only patch from #30 I actually get the following (expected) test fail:

1) Drupal\Tests\field_ui\FunctionalJavascript\EntityDisplayTest::testHiddenExtraFields
TypeError: Cannot read properties of undefined (reading 'value')
    at Drupal.fieldUIDisplayOverview.field.regionChange (http://drupal-core.localhost:8888/core/modules/field_ui/field_ui.js?v=10.1.0-dev:371:51)
    at Drupal.tableDrag.onDrop (http://drupal-core.localhost:8888/core/modules/field_ui/field_ui.js?v=10.1.0-dev:192:42)
    at Drupal.tableDrag.dropRow (http://drupal-core.localhost:8888/core/misc/tabledrag.js?v=10.1.0-dev:861:12)
    at HTMLDocument.<anonymous> (http://drupal-core.localhost:8888/core/misc/tabledrag.js?v=10.1.0-dev:257:57)
    at HTMLDocument.dispatch (http://drupal-core.localhost:8888/core/assets/vendor/jquery/jquery.min.js?v=3.6.3:2:43336)
    at y.handle (http://drupal-core.localhost:8888/core/assets/vendor/jquery/jquery.min.js?v=3.6.3:2:41320)

This is patch from #25 with appropriate test methods in core/**/FunctionalJavascript/EntityDisplayTest.php and core/**/FunctionalJavascriptTests/Theme/ClaroEntityDisplayTest.php.

3305706-uncaught-typeerror-35-TEST_ONLY_plus_3331835.patch also includes the necessary bits of #3331835-8: Some JavaScript errors are not recorded during test runs.

From my point of view this is ready now. I guess we need to come to a decision on whether this issue should be blocked on #3331835: Some JavaScript errors are not recorded during test runs or whether it can be committed despite the fact that the test-only patch currently is not resulting in the expected test fail.

@smustgrave pinged me to flag the issue here w.r.t the testing. I discussed it with @xjm to review the three possible approaches:

* Commit with the test that doesn't fail and fix the testing bug separately
* Block this on the testing bug
* Commit the fix here and move the tests to a followup postponed on the testing bug

@xjm felt that because this is major, the third option seems like the best approach. She also bumped the testing bug to critical.

So can we get a followup for the tests that is postponed on this and the testing bug, and a patch here with just the fixes.

Because this is a bug it can be committed right back to 9.5, so we'll need a 9.5 patch with the compiled JS (non ES6) too I think.

Adding more keywords to the issue title, found a couple of duplicates and related them back to this one

Thanks @larowlan!

So @znerol can you upload the fix only from #35. Would do it myself but then I can't be reviewer.

Tagging for followup to create that postponed ticket for the tests.

Patches for 10.1.x and 9.5.x without tests.

Follow-up #3332148: [PP-1] Test cases for extra field drag&drop issue

- Code changes make sense.
- TestBot agrees in #35 where we still have the tests.
- #42 contains only the fix itself, not the tests, as requested in #39.
- The d9 patch in #42 contains the fix only, whilst doing the es6-JS-jig.

All in all nice work and sleuthing @znerol!
RTBC for me.

Adding credit from #3295724: Unable to move fields to an empty area in the display mode. which I closed as a duplicate

+++ b/core/modules/field_ui/field_ui.js
@@ -365,10 +365,14 @@
-            : this.$pluginSelect.find('option')[0].value;
+            : pluginSelect;

I think we can just write this as this.$pluginSelect.find('option')[0]?.value for the D10 version where we don't have to support IE11 in D10+

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operat...

We still need this dance for the D9 version though.

@larowlan: I think you're spot on (per usual) and this approach was tried in #23.

Sadly, as @znerol already discribed in #25, scripts/dev/commit-code-check.sh isn't ready for optional chaining in JavaScript (yet?). See https://www.drupal.org/pift-ci-job/2561443 for the "TestBot-Says-No!" result.

I'll open a follow-up so that can be addressed, but I think, since this is a Major, we should get this in ASAP and not postpone this on our tooling not being up to scratch. So back to RTBC. Happy to be educated on why this was a potential bad status change.

INSTA-EDIT: Opened #3335653: commit-code-check.sh doesn't allow optional chaining in JavaScript for the commit-code-check.sh woes.

Yep I agree with that approach - will pick this up tomorrow

Probably too late in larowlan-land by now *shakes fist in anger at time zones*, but it would be nice if we can get this Major one in.

Adding issue credits, crediting @SandeepSingh199 as I disagree with @smustgrave and feel they were trying to help the issue move forward

Crediting @smustgrave and @Spokje for keeping the issue moving, issue summary updates, filing follow ups etc

Committed to 10.1.x and backported to 10.0.x

Committed to 9.5.x

Sorry for taking a bit longer, yesterday was busy.

Thanks @larowlan

Sorry for taking a bit longer, yesterday was busy.

No worries, that's why I desperately try to avoid terms like "tomorrow" without adding "-ish" to cover my behind... ;)

Closed #3295701: Uncaught TypeError: Cannot read properties of undefined (reading 'value') as duplicate