From bf00810e1f5a7164d74ad66f3d03e813d81dfa25 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Wed, 8 Jun 2016 12:10:01 +1200 Subject: [PATCH 1/3] BUG Fix buttonClicked() error Fixes #3208 --- forms/Form.php | 12 +++++++++--- tests/forms/FormTest.php | 12 +++++++++--- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/forms/Form.php b/forms/Form.php index 0714aceef..eac7378a5 100644 --- a/forms/Form.php +++ b/forms/Form.php @@ -873,7 +873,7 @@ class Form extends RequestHandler { /** * Set the target of this form to any value - useful for opening the form contents in a new window or refreshing * another frame - * + * * @param string|FormTemplateHelper */ public function setTemplateHelper($helper) { @@ -1635,11 +1635,17 @@ class Form extends RequestHandler { * @return FormAction */ public function buttonClicked() { - foreach($this->actions->dataFields() as $action) { - if($action->hasMethod('actionname') && $this->buttonClickedFunc == $action->actionName()) { + $actions = $this->actions->dataFields(); + if(!$actions) { + return null; + } + + foreach($actions as $action) { + if($action instanceof FormAction && $this->buttonClickedFunc == $action->actionName()) { return $action; } } + return null; } /** diff --git a/tests/forms/FormTest.php b/tests/forms/FormTest.php index 27814a466..dfcd4fe6e 100644 --- a/tests/forms/FormTest.php +++ b/tests/forms/FormTest.php @@ -19,7 +19,7 @@ class FormTest extends FunctionalTest { Config::inst()->update('Director', 'rules', array( 'FormTest_Controller' => 'FormTest_Controller' )); - + // Suppress themes Config::inst()->remove('SSViewer', 'theme'); } @@ -324,7 +324,7 @@ class FormTest extends FunctionalTest { public function testDisableSecurityTokenAcceptsSubmissionWithoutToken() { SecurityToken::enable(); $expectedToken = SecurityToken::inst()->getValue(); - + $response = $this->get('FormTest_ControllerWithSecurityToken'); // can't use submitForm() as it'll automatically insert SecurityID into the POST data $response = $this->post( @@ -537,6 +537,12 @@ class FormTest extends FunctionalTest { $this->assertEquals('bar', $attrs['foo']); } + public function testButtonClicked() { + $form = $this->getStubForm(); + $action = $form->buttonClicked(); + $this->assertNull($action); + } + public function testAttributesHTML() { $form = $this->getStubForm(); @@ -622,7 +628,7 @@ class FormTest extends FunctionalTest { $formData = $form->getData(); $this->assertEmpty($formData['ExtraFieldCheckbox']); } - + protected function getStubForm() { return new Form( new FormTest_Controller(), From 8f2953f1a03ebbe7f1fefcb3637001d2320409fc Mon Sep 17 00:00:00 2001 From: Giancarlo Di Massa Date: Thu, 14 Jul 2016 16:35:51 +0200 Subject: [PATCH 2/3] Quick fix for HTTP/2.0 on OSX/Safari v9.1.1 --- admin/javascript/LeftAndMain.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/admin/javascript/LeftAndMain.js b/admin/javascript/LeftAndMain.js index 6295e69ae..791071744 100644 --- a/admin/javascript/LeftAndMain.js +++ b/admin/javascript/LeftAndMain.js @@ -141,7 +141,7 @@ jQuery.noConflict(); var msg = (xhr.getResponseHeader('X-Status')) ? xhr.getResponseHeader('X-Status') : xhr.statusText, reathenticate = xhr.getResponseHeader('X-Reauthenticate'), msgType = (xhr.status < 200 || xhr.status > 399) ? 'bad' : 'good', - ignoredMessages = ['OK', 'success']; + ignoredMessages = ['OK', 'success', 'HTTP/2.0 200']; // Enable reauthenticate dialog if requested if(reathenticate) { From b3fea3723fae822068a6b761a682011b0970fff5 Mon Sep 17 00:00:00 2001 From: Loz Calver Date: Fri, 15 Jul 2016 10:11:47 +0100 Subject: [PATCH 3/3] FIX: Fixes support for "inline" form actions (fixes #2534) --- forms/Form.php | 51 ++++++++++++++++++++++++++-------------- tests/forms/FormTest.php | 48 +++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 17 deletions(-) diff --git a/forms/Form.php b/forms/Form.php index eac7378a5..cea8d311d 100644 --- a/forms/Form.php +++ b/forms/Form.php @@ -416,7 +416,8 @@ class Form extends RequestHandler { $this->controller->hasMethod($funcName) && !$this->controller->checkAccessAction($funcName) // If a button exists, allow it on the controller - && !$this->actions->dataFieldByName('action_' . $funcName) + // buttonClicked() validates that the action set above is valid + && !$this->buttonClicked() ) { return $this->httpError( 403, @@ -475,16 +476,28 @@ class Form extends RequestHandler { * @return bool */ public function checkAccessAction($action) { - return ( - parent::checkAccessAction($action) - // Always allow actions which map to buttons. See httpSubmission() for further access checks. - || $this->actions->dataFieldByName('action_' . $action) - // Always allow actions on fields - || ( - $field = $this->checkFieldsForAction($this->Fields(), $action) - && $field->checkAccessAction($action) - ) - ); + if (parent::checkAccessAction($action)) { + return true; + } + + // Always allow actions which map to buttons. See httpSubmission() for further access checks. + $fields = $this->fields->dataFields() ?: array(); + $actions = $this->actions->dataFields() ?: array(); + + $fieldsAndActions = array_merge($fields, $actions); + foreach ($fieldsAndActions as $fieldOrAction) { + if ($fieldOrAction instanceof FormAction && $fieldOrAction->actionName() === $action) { + return true; + } + } + + // Always allow actions on fields + $field = $this->checkFieldsForAction($this->Fields(), $action); + if ($field && $field->checkAccessAction($action)) { + return true; + } + + return false; } /** @@ -1635,16 +1648,20 @@ class Form extends RequestHandler { * @return FormAction */ public function buttonClicked() { - $actions = $this->actions->dataFields(); - if(!$actions) { + $fields = $this->fields->dataFields() ?: array(); + $actions = $this->actions->dataFields() ?: array(); + + if(!$actions && !$fields) { return null; } - foreach($actions as $action) { - if($action instanceof FormAction && $this->buttonClickedFunc == $action->actionName()) { - return $action; + $fieldsAndActions = array_merge($fields, $actions); + foreach ($fieldsAndActions as $fieldOrAction) { + if ($fieldOrAction instanceof FormAction && $this->buttonClickedFunc === $fieldOrAction->actionName()) { + return $fieldOrAction; } } + return null; } @@ -1657,7 +1674,7 @@ class Form extends RequestHandler { public function defaultAction() { if($this->hasDefaultAction && $this->actions) { return $this->actions->First(); - } + } } /** diff --git a/tests/forms/FormTest.php b/tests/forms/FormTest.php index dfcd4fe6e..ebe740bb3 100644 --- a/tests/forms/FormTest.php +++ b/tests/forms/FormTest.php @@ -541,6 +541,54 @@ class FormTest extends FunctionalTest { $form = $this->getStubForm(); $action = $form->buttonClicked(); $this->assertNull($action); + + $controller = new FormTest_Controller(); + $form = $controller->Form(); + $request = new SS_HTTPRequest('POST', 'FormTest_Controller/Form', array(), array( + 'Email' => 'test@test.com', + 'SomeRequiredField' => 1, + 'action_doSubmit' => 1 + )); + + $form->httpSubmission($request); + $button = $form->buttonClicked(); + $this->assertInstanceOf('FormAction', $button); + $this->assertEquals('doSubmit', $button->actionName()); + + $form = new Form( + $controller, + 'Form', + new FieldList(new FormAction('doSubmit', 'Inline action')), + new FieldList() + ); + $form->disableSecurityToken(); + $request = new SS_HTTPRequest('POST', 'FormTest_Controller/Form', array(), array( + 'action_doSubmit' => 1 + )); + + $form->httpSubmission($request); + $button = $form->buttonClicked(); + $this->assertInstanceOf('FormAction', $button); + $this->assertEquals('doSubmit', $button->actionName()); + } + + public function testCheckAccessAction() { + $controller = new FormTest_Controller(); + $form = new Form( + $controller, + 'Form', + new FieldList(), + new FieldList(new FormAction('actionName', 'Action')) + ); + $this->assertTrue($form->checkAccessAction('actionName')); + + $form = new Form( + $controller, + 'Form', + new FieldList(new FormAction('inlineAction', 'Inline action')), + new FieldList() + ); + $this->assertTrue($form->checkAccessAction('inlineAction')); } public function testAttributesHTML() {