silverstripe / silverstripe-cms

Silverstripe CMS - this is a module for Silverstripe Framework rather than a standalone app. Use https://github.com/silverstripe/silverstripe-installer/ to set this up.
http://silverstripe.org/
BSD 3-Clause "New" or "Revised" License
515 stars 332 forks source link

[2012-11-09] CMSEditor dialogs - MediForm and LinkForm broken in ModelAdmin #587

Closed silverstripe-issues closed 11 years ago

silverstripe-issues commented 11 years ago

created by: @icecaster (tim) created at: 2012-11-09 original ticket: http://open.silverstripe.org/ticket/8013


when clicking th image icon in the editor, a 500 error gets thrown as Modeladmin does not recognise "EditorToolbar" as valid action

silverstripe-issues commented 11 years ago

comment by: @icecaster (tim) created at: 2012-11-09


Ok, here is a fix:

In framework/forms/HtmlEditorField.php in class "HtmlEditorField_Toolbar" @ line 269

replace

public function forTemplate() {
    return sprintf(
        '<div id="cms-editor-dialogs" data-url-linkform="%s" data-url-mediaform="%s"></div>',
        Controller::join_links($this->controller->Link($this->name), 'LinkForm', 'forTemplate'),
        Controller::join_links($this->controller->Link($this->name), 'MediaForm', 'forTemplate')
    );
}

with

public function forTemplate() {
    return sprintf(
        '<div id="cms-editor-dialogs" data-url-linkform="%s" data-url-mediaform="%s"></div>',
        Controller::join_links($this->controller->Link(), $this->name, 'LinkForm', 'forTemplate'),
        Controller::join_links($this->controller->Link(), $this->name, 'MediaForm', 'forTemplate')
    );
}

The problem is, that the 'action' parameter in ModelAdmin::Link() in fact is the currently handled model, rather than the action. By appending the action to the returned link, we get the currently managed model returned from Modeladmin and can append our action.

I have confirmed this working in model admin as well as CMS.

Any objections?

silverstripe-issues commented 11 years ago

comment by: @JayDevlin (Devlin) created at: 2012-11-09


I find this solution too obvious. In my opinion ModelAdmin should provide the modelClass in Link -- like in 2.4.x -- because the parameter name of Link() is $action, not $model. With fixing this in HtmlEditorField_Toolbar only, the issue still exists for everybody with a custom form field with a same functionality.

silverstripe-issues commented 11 years ago

comment by: @JayDevlin (Devlin) created at: 2012-11-09


I would suggest the following change in ModelAdmin.php

public function Link($action = null) {
    $models = $this->getManagedModels();
    if(!array_key_exists($action, $models)) {
        $action = Controller::join_links($this->sanitiseClassName($this->modelClass), $action);
    }

    //if(!$action) $action = $this->sanitiseClassName($this->modelClass);
    return parent::Link($action);
}
silverstripe-issues commented 11 years ago

comment by: @chillu (ischommer) created at: 2012-11-15


Just for posterity, the original suggestion has been committed with https://github.com/silverstripe/sapphire/commit/f0f5dcb966da9123e88dc1f8acc71065cfd7c8d8.

Reopening the issue for now, @Devlin could you submit a pull request?

chillu commented 11 years ago

I've verified that both images and links can be inserted in a 3.1 ModelAdmin.