Problem/Motivation

The Content Translation module includes a Drupal behavior in core/modules/content_translation
/content_translation.admin.js.

This behavior at some point processes the received $context to locate input elements:

$fields = $context.find(`input[name^="${field}"]`);

However, it doesn't check if $fields array is empty, causing JS errors when it is indeed empty:

Uncaught TypeError: Cannot read properties of undefined (reading 'matches')

Steps to reproduce

  1. Install a fresh Drupal site
  2. Enable content translation (drush en content_translation)
  3. Go to "Content language and translation" at /admin/config/regional/content-language

The JS error should be displayed in the browser console.

Proposed resolution

Check if $fields is empty.

Remaining tasks

User interface changes

None.

API changes

None.

Data model changes

None.

Release notes snippet

Not needed.

Issue fork drupal-3410011

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

tunic created an issue. See original summary.

tunic’s picture

Status: Active » Needs review
smustgrave’s picture

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

Could we get a test case showing the issue.

tunic’s picture

Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new14.97 MB

We could add a test but I'm not sure if we should. The issue is a missing conditional safeguard to avoid using an empty array as it is not empty. I'm not convinced we should add a new test to core, adding a few seconds to each test run, for this simple issue.

This is the original code:

      if (options && options.dependent_selectors) {
        Object.keys(options.dependent_selectors).forEach((field) => {
          $fields = $context.find(`input[name^="${field}"]`);

          const dependentColumns = options.dependent_selectors[field];

          $fields.on('change', fieldsChangeHandler($fields, dependentColumns));
          Drupal.behaviors.contentTranslationDependentOptions.check(
            $fields,
            dependentColumns,
          );

$fields is passed to Drupal.behaviors.contentTranslationDependentOptions.check but it is empty.

I've uploaded a video with the issue, showing the problem in Simplytestme and the fix applying the patch.
See https://www.drupal.org/files/issues/2023-12-20/20231220-Drupal-issue-341...

Of course, we can still add a test, but I would say we should test JS is not broken in the /admin/config/regional/content-language page. That would be a more general test. Not sure if this is the proper place, it could be better to use this issue to add it.

smustgrave’s picture

Status: Needs review » Needs work

Sorry for the delay.

I think a generic javascript test should be fine. I believe WebDriverTestBase has an ability to test for console errors. Is that what you meant at least in the last sentence?

tunic’s picture

StatusFileSize
new56.51 KB
new18.66 KB

The test was trickier than expected. I needed to install the standard profile to get a failing test. Enabling content translation or the minimal profile was not enough. I guess the problem are the entities created.

This is the "Content language and translation" page (/admin/config/regional/content-language) with the standard profile:

And this enabling only the content translation module:

tunic’s picture

Status: Needs work » Needs review

As expected test passes but the "Test-only" job fails, demonstrating the issue.

See https://git.drupalcode.org/issue/drupal-3410011/-/pipelines/70540

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

Good work on the test! Did confirm the browser error following those steps, and that MR fixed it.

kevinkonsorr’s picture

I found that changes in the core/modules/content_translation/content_translation.admin.js file break the content translation page on 10.2.

Line 92 was falsely changes from
if (!$input.is(':checked')) {
to
if (input.checked) {
removing the negotion here.

I attached a patch to quickly address this and to use the content translation page again.

kevinkonsorr’s picture

tunic’s picture

#10, I think the problem you are pointing it is a different issue. If I apply that patch to 10.2 without the fix in the MR the error described in this issue are still present.

I would say a new issue has to be created.

kevinkonsorr’s picture

I think I was not clear enough, sorry.
That patch is to use in addition to the fix in this very issue. As it is already part of 10.2.2,
I simply added this patch to fix the wrong if-clause (mentioned in #10) that was only brought by this very issue here:
https://git.drupalcode.org/issue/drupal-3410011/-/commit/cd9fed87f0568d9...

kevinkonsorr’s picture

I checked deeper and it seems that issue was created here.
https://www.drupal.org/project/drupal/issues/3238849

One of those commits it only part of the fork for 3410011 – I guess that I mixed that up.

kevinkonsorr’s picture

tunic’s picture

Yes, the commit of the error you mention was introduced in the MR of this issue when a rebase was done, adding all commits committed the dev branch.

So we are good here, and still RTBC-

quietone’s picture

Status: Reviewed & tested by the community » Needs work

I'm triaging RTBC issues. I read the IS, the comments and the MR. I didn't find any unanswered questions.

\This is a nice wee fix and with a test!

However, setting to NW for the test to use the testing profile. This is because we have working to reduce the usages of our resources. I expect the tests in the content_translation module can help with this.

#3270566: [meta] Convert remaining tests using Standard profile to use Testing profile instead

sleitner’s picture

Component: config_translation.module » content_translation.module
nod_’s picture

Issue tags: +JavaScript
sleitner’s picture

Status: Needs work » Closed (duplicate)
Issue tags: -JavaScript +JavaScript
Related issues: +#3416304: Javascript warning from content language and translation page

I think this is a duplicate of #3416304: Javascript warning from content language and translation page. It is fixed in 10.3.x-dev