Additional Breaking Change in FormAction since 3.1.10?

39 views
Skip to first unread message

Stefan

unread,
Apr 14, 2015, 4:23:43 AM4/14/15
to silverst...@googlegroups.com
Hi,

during an upgrade of a SilverStripe installation from <3.1.10 to 3.1.12, we stumbled across a bug which was hard to locate.
In https://github.com/silverstripe/silverstripe-framework/pull/3894 a change concerning the title of a FormAction took place. But actually also a (for us major) api fix was applied.
It seems that until this fix, the $form parameter of FormAction was never used, it was passed to the FormField constructor, which did not handle it.
In the above commits, this was fixed by adding a $this->setForm($form);

In our code where we worked with a very modified version of the GridFieldDetailForm for Pages where we used code like the following to add well known actions:
FormAction::create('doUnpublish', _t('SiteTree.BUTTONUNPUBLISH', 'Unpublish'), 'delete')

It seems that we copied this line from core classes, because in the current SiteTree class, there is a nearly equal line of code:
FormAction::create('unpublish', _t('SiteTree.BUTTONUNPUBLISH', 'Unpublish'), 'delete')

The latter case seems to work, but our line broke the CMS. Why?
Since 3.1.10 it sets the FormField->$form to a string, which caused a
PHP Fatal error:  Call to a member function FormName() on a non-object in /vagrant/silverstripe/framework/forms/FormField.php on line 167

I didn't want to create an issue without discussing this change here.
At least there has to be a fix for the SiteTree class, but maybe the FormAction's $form param should be handled backwards-compatible (eg. by checking for a Form instance)?

Thanks

Stefan

Stefan

unread,
Apr 14, 2015, 4:28:37 AM4/14/15
to silverst...@googlegroups.com
As I can see there are two lines in SiteTree that contain this third parameter ($value, analog to FormField). They seem to be very old ;)
Unfortunately we copied this line into our modifications.
Reply all
Reply to author
Forward
0 new messages