FriendsOfSymfony / FOSCKEditorBundle

Provides a CKEditor integration for your Symfony project.
Other
518 stars 83 forks source link

[Config] Default configuration overriding #209

Open fsoedjede opened 4 years ago

fsoedjede commented 4 years ago

Environment

Symfony 4.4.2 PHP 7.3

Subject

I think there is an issue with configuration overriding. When I set a default configuration, the overriding applies to only first level in the array. I want to add parameters to filebrowserImageBrowseRouteParameters but currently I need to redefine all keys of filebrowserImageBrowseRouteParameters in order to make it work

Steps to reproduce

  1. Create a form
    
    use FOS\CKEditorBundle\Form\Type\CKEditorType;
    use Symfony\Component\Form\AbstractType;
    use Symfony\Component\Form\FormBuilderInterface;

class NewsType extends AbstractType { public function buildForm(FormBuilderInterface $builder, array $options) { $builder ->add('summary',CKEditorType::class, array( 'required' => false, 'config_name' => 'admin', 'config' => [ 'filebrowserImageBrowseRouteParameters' => [ 'context' => 'news' ], 'filebrowserUploadRouteParameters' => [ 'context' => 'news' ], ] )); } }


2. Define default config:

```yaml
fos_ck_editor:
    default_config: admin
    configs:
        admin:
            filebrowserBrowseRoute: admin_media_browser
            filebrowserImageBrowseRoute: admin_media_browser
            filebrowserImageBrowseRouteParameters:
                provider: image
                action: browser
            filebrowserUploadMethod: form
            filebrowserUploadRoute: admin_media_upload
            filebrowserUploadRouteParameters:
                provider: image
                action: upload

Expected results

At the end of FOS\CKEditorBundle\Form\Type\CKEditorType::resolveConfig,

$config['filebrowserImageBrowseRouteParameters'] should contain 3 items: provider, action and context

Actual results

$config['filebrowserImageBrowseRouteParameters'] contains only one item: context

The upload does not work in that context.

How to fix

In the file https://github.com/FriendsOfSymfony/FOSCKEditorBundle/blob/2.x/src/Form/Type/CKEditorType.php#L72

Prevously:

$config = array_merge($this->configuration->getConfig($options['config_name']), $config);

Proposal:

$config = array_merge_recursive($this->configuration->getConfig($options['config_name']), $config);

There may be a BC break if that change is made

Regards

fsoedjede commented 4 years ago

Bad idea. it does not now well. Maybe we can add condition to merge array items?