Problem/Motivation

As in the title, the strpos function is throwing the error because the given int key (of the form element) which is common in case
of the sequence form elements (keyed by int's - 0, 1, 2, 3...).

The current code looks like this:

      foreach ($plugins_config_form as $key => &$value) {
        if (is_array($value) && strpos($key, '#') === FALSE) {
          _add_ajax_listeners_to_plugin_inputs($value);
        }
      }

Steps to reproduce

Define any form elements in the plugin which will have some sequence. You can base it on the example schema:

        header:
          type: sequence
          label: 'Header'
          sequence:
            type: mapping
            mapping:
              html:
                type: string
                label: 'HTML'
              css:
                type: string
                label: 'CSS'
              type:
                type: string
                label: 'Type'

The error is thrown when trying to edit the given text format (ex. full HTML - /admin/config/content/formats/manage/full_html).
The plugin probably needs to be added first to the toolbar (so it could try to load the form)

Proposed resolution

There are few options, the simplest and still quite clean is to cast $key to string. I believe it might be even faster than adding additional checking for the int.
Still, we can do it these ways:
1.

      foreach ($plugins_config_form as $key => &$value) {
        if (is_array($value) && strpos((string) $key, '#') === FALSE) {
          _add_ajax_listeners_to_plugin_inputs($value);
        }
      }

2.

      foreach ($plugins_config_form as $key => &$value) {
        if (is_array($value) && !is_int($key) && strpos($key, '#') === FALSE) {
          _add_ajax_listeners_to_plugin_inputs($value);
        }
      }

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3268272

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

sayco created an issue. See original summary.

sayco’s picture

Status: Active » Needs review
wim leers’s picture

Status: Needs review » Reviewed & tested by the community

Reminds me of #3264148: remove inner _add_ajax_listeners_to_plugin_inputs and _add_attachments_to_editor_update_response function definitions, but it's completely different.

Requiring test coverage for this seems over the top. It's an obvious hardening 👍

Thank you!

  • catch committed e605d0d on 10.0.x
    Issue #3268272 by sayco: TypeError: strpos(): Argument #1 ($haystack)...

  • catch committed b77aba7 on 9.4.x
    Issue #3268272 by sayco: TypeError: strpos(): Argument #1 ($haystack)...

  • catch committed 12210b1 on 9.3.x
    Issue #3268272 by sayco: TypeError: strpos(): Argument #1 ($haystack)...
catch’s picture

Version: 10.0.x-dev » 9.3.x-dev
Status: Reviewed & tested by the community » Fixed

Only question with me is whether we need a test ckeditor plugin with this form structure to exercise any other parts of the code that might be struggling with this. However there's no added code path here so I agree it's OK to commit as-is. Assume this is blocking contrib ckeditor plugins, which overall are going to give us more coverage than the test here would.

If we find another similar issue and wished we'd added test coverage here, we can add test coverage there.

Committed/pushed to 10.0.x and cherry-picked to 9.4.x and 9.3.x, thanks!

Status: Fixed » Closed (fixed)

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