diff --git a/admin/javascript/LeftAndMain.EditForm.js b/admin/javascript/LeftAndMain.EditForm.js index 131432642..b73c45e24 100644 --- a/admin/javascript/LeftAndMain.EditForm.js +++ b/admin/javascript/LeftAndMain.EditForm.js @@ -2,7 +2,7 @@ * File: LeftAndMain.EditForm.js */ (function($) { - + // Can't bind this through jQuery window.onbeforeunload = function(e) { var form = $('.cms-edit-form'); @@ -14,7 +14,7 @@ /** * Class: .cms-edit-form - * + * * Base edit form, provides ajaxified saving * and reloading itself through the ajax return values. * Takes care of resizing tabsets within the layout container. @@ -25,20 +25,20 @@ * * @name ss.Form_EditForm * @require jquery.changetracker - * + * * Events: * ajaxsubmit - Form is about to be submitted through ajax * validate - Contains validation result * load - Form is about to be loaded through ajax */ - $('.cms-edit-form').entwine(/** @lends ss.Form_EditForm */{ + $('.cms-edit-form').entwine(/** @lends ss.Form_EditForm */{ /** * Variable: PlaceholderHtml * (String_ HTML text to show when no form content is chosen. * Will show inside the <form> tag. */ PlaceholderHtml: '', - + /** * Variable: ChangeTrackerOptions * (Object) @@ -46,7 +46,15 @@ ChangeTrackerOptions: { ignoreFieldSelector: '.no-change-track, .ss-upload :input, .cms-navigator :input' }, - + + /** + * Variable: ValidationErrorShown + * Boolean for tracking whether a validation error has been already been shown. Used because tabs can + * sometimes be inadvertently initialised multiple times, but we don't want duplicate messages + * (Boolean) + */ + ValidationErrorShown: false, + /** * Constructor: onmatch */ @@ -61,13 +69,13 @@ // See the following page for demo and explanation of the Firefox bug: // http://www.ryancramer.com/journal/entries/radio_buttons_firefox/ this.attr("autocomplete", "off"); - + this._setupChangeTracker(); // Catch navigation events before they reach handleStateChange(), // in order to avoid changing the menu state if the action is cancelled by the user // $('.cms-menu') - + // Optionally get the form attributes from embedded fields, see Form->formHtmlContent() for(var overrideAttr in {'action':true,'method':true,'enctype':true,'name':true}) { var el = this.find(':input[name='+ '_form_' + overrideAttr + ']'); @@ -77,22 +85,43 @@ } } + // Reset error display + this.setValidationErrorShown(false); + // TODO // // Rewrite # links // html = html.replace(/(<a[^>]+href *= *")#/g, '$1' + window.location.href.replace(/#.*$/,'') + '#'); - // + // // // Rewrite iframe links (for IE) // html = html.replace(/(<iframe[^>]*src=")([^"]+)("[^>]*>)/g, '$1' + $('base').attr('href') + '$2$3'); - + + this._super(); + }, + 'from .cms-tabset': { + onafterredrawtabs: function () { // Show validation errors if necessary if(this.hasClass('validationerror')) { // Ensure the first validation error is visible var tabError = this.find('.message.validation, .message.required').first().closest('.tab'); $('.cms-container').clearCurrentTabState(); // clear state to avoid override later on - tabError.closest('.ss-tabset').tabs('option', 'active', tabError.index('.tab')); + + // Attempt #1: Look for nearest .ss-tabset (usually nested deeper underneath a .cms-tabset). + var $tabSet = tabError.closest('.ss-tabset'); + + // Attempt #2: Next level in tab-ception, try to select the tab within this higher level .cms-tabset if possible + if (!$tabSet.length) { + $tabSet = tabError.closest('.cms-tabset'); } - this._super(); + if ($tabSet.length) { + $tabSet.tabs('option', 'active', tabError.index('.tab')); + } else if (!this.getValidationErrorShown()) { + // Ensure that this error message popup won't be added more than once + this.setValidationErrorShown(true); + errorMessage(ss.i18n._t('ModelAdmin.VALIDATIONERROR', 'Validation Error')); + } + } + } }, onremove: function() { this.changetracker('destroy'); @@ -106,7 +135,7 @@ }, redraw: function() { if(window.debug) console.log('redraw', this.attr('class'), this.get(0)); - + // Force initialization of tabsets to avoid layout glitches this.add(this.find('.cms-tabset')).redrawTabs(); this.find('.cms-content-header').redraw(); @@ -120,19 +149,19 @@ // full <form> tag by any ajax updates they won't automatically reapply this.changetracker(this.getChangeTrackerOptions()); }, - + /** * Function: confirmUnsavedChanges - * + * * Checks the jquery.changetracker plugin status for this form, * and asks the user for confirmation via a browser dialog if changes are detected. * Doesn't cancel any unload or form removal events, you'll need to implement this based on the return * value of this message. - * + * * If changes are confirmed for discard, the 'changed' flag is reset. - * + * * Returns: - * (Boolean) FALSE if the user wants to abort with changes present, TRUE if no changes are detected + * (Boolean) FALSE if the user wants to abort with changes present, TRUE if no changes are detected * or the user wants to discard them. */ confirmUnsavedChanges: function() { @@ -150,7 +179,7 @@ /** * Function: onsubmit - * + * * Suppress submission unless it is handled through ajaxSubmit(). */ onsubmit: function(e, button) { @@ -167,20 +196,20 @@ /** * Function: validate - * + * * Hook in (optional) validation routines. * Currently clientside validation is not supported out of the box in the CMS. - * + * * Todo: * Placeholder implementation - * + * * Returns: * {boolean} */ validate: function() { var isValid = true; this.trigger('validate', {isValid: isValid}); - + return isValid; }, /* @@ -210,7 +239,7 @@ } }, /* - * Track focus on treedropdownfields. + * Track focus on treedropdownfields. */ 'from .cms-edit-form .treedropdown *': { onfocusin: function(e){ @@ -240,12 +269,12 @@ */ saveFieldFocus: function(selected){ if(typeof(window.sessionStorage)=="undefined" || window.sessionStorage === null) return; - + var id = $(this).attr('id'), focusElements = []; focusElements.push({ - id:id, + id:id, selected:selected }); @@ -254,7 +283,7 @@ window.sessionStorage.setItem(id, JSON.stringify(focusElements)); } catch(err) { if (err.code === DOMException.QUOTA_EXCEEDED_ERR && window.sessionStorage.length === 0) { - // If this fails we ignore the error as the only issue is that it + // If this fails we ignore the error as the only issue is that it // does not remember the focus state. // This is a Safari bug which happens when private browsing is enabled. return; @@ -272,7 +301,7 @@ */ restoreFieldFocus: function(){ if(typeof(window.sessionStorage)=="undefined" || window.sessionStorage === null) return; - + var self = this, hasSessionStorage = (typeof(window.sessionStorage)!=="undefined" && window.sessionStorage), sessionData = hasSessionStorage ? window.sessionStorage.getItem(this.attr('id')) : null, @@ -330,9 +359,9 @@ if(scrollY > $(window).height() / 2){ self.find('.cms-content-fields').scrollTop(scrollY); } - + } else { - // If session storage is not supported or there is nothing stored yet, focus on the first input + // If session storage is not supported or there is nothing stored yet, focus on the first input this.focusFirstInput(); } }, @@ -349,7 +378,7 @@ /** * Class: .cms-edit-form .Actions :submit - * + * * All buttons in the right CMS form go through here by default. * We need this onclick overloading because we can't get to the * clicked button from a form.onsubmit event. @@ -359,7 +388,7 @@ * Function: onclick */ onclick: function(e) { - // Confirmation on delete. + // Confirmation on delete. if( this.hasClass('gridfield-button-delete') && !confirm(ss.i18n._t('TABLEFIELD.DELETECONFIRMMESSAGE')) diff --git a/admin/javascript/LeftAndMain.js b/admin/javascript/LeftAndMain.js index c63787bd0..23d141c77 100644 --- a/admin/javascript/LeftAndMain.js +++ b/admin/javascript/LeftAndMain.js @@ -1450,6 +1450,8 @@ jQuery.noConflict(); } } }); + + this.trigger('afterredrawtabs'); }, /** diff --git a/core/startup/ErrorControlChain.php b/core/startup/ErrorControlChain.php index 86a149e45..8925c3239 100644 --- a/core/startup/ErrorControlChain.php +++ b/core/startup/ErrorControlChain.php @@ -77,13 +77,35 @@ class ErrorControlChain { */ public function setSuppression($suppression) { $this->suppression = (bool)$suppression; - // Don't modify errors unless handling fatal errors, and if errors were - // originally allowed to be displayed. - if ($this->handleFatalErrors && $this->originalDisplayErrors) { - ini_set('display_errors', !$suppression); + // If handling fatal errors, conditionally disable, or restore error display + // Note: original value of display_errors could also evaluate to "off" + if ($this->handleFatalErrors) { + if($suppression) { + $this->setDisplayErrors(0); + } else { + $this->setDisplayErrors($this->originalDisplayErrors); + } } } + /** + * Set display_errors + * + * @param mixed $errors + */ + protected function setDisplayErrors($errors) { + ini_set('display_errors', $errors); + } + + /** + * Get value of display_errors ini value + * + * @return mixed + */ + protected function getDisplayErrors() { + return ini_get('display_errors'); + } + /** * Add this callback to the chain of callbacks to call along with the state * that $error must be in this point in the chain for the callback to be called @@ -178,7 +200,7 @@ class ErrorControlChain { register_shutdown_function(array($this, 'handleFatalError')); $this->handleFatalErrors = true; - $this->originalDisplayErrors = ini_get('display_errors'); + $this->originalDisplayErrors = $this->getDisplayErrors(); $this->setSuppression($this->suppression); $this->step(); @@ -202,7 +224,7 @@ class ErrorControlChain { else { // Now clean up $this->handleFatalErrors = false; - ini_set('display_errors', $this->originalDisplayErrors); + $this->setDisplayErrors($this->originalDisplayErrors); } } } diff --git a/forms/gridfield/GridFieldExportButton.php b/forms/gridfield/GridFieldExportButton.php index a79dca6c6..307399e20 100644 --- a/forms/gridfield/GridFieldExportButton.php +++ b/forms/gridfield/GridFieldExportButton.php @@ -146,7 +146,7 @@ class GridFieldExportButton implements GridField_HTMLProvider, GridField_ActionP } else { $value = $gridField->getDataFieldValue($item, $columnSource); - if(!$value) { + if($value === null) { $value = $gridField->getDataFieldValue($item, $columnHeader); } } diff --git a/model/fieldtypes/DBLocale.php b/model/fieldtypes/DBLocale.php index c509cf79d..cf279af55 100644 --- a/model/fieldtypes/DBLocale.php +++ b/model/fieldtypes/DBLocale.php @@ -9,7 +9,7 @@ */ class DBLocale extends Varchar { - public function __construct($name, $size = 16) { + public function __construct($name = null, $size = 16) { parent::__construct($name, $size); } diff --git a/tests/core/startup/ErrorControlChainTest.php b/tests/core/startup/ErrorControlChainTest.php index a6c2c2496..4f550c548 100644 --- a/tests/core/startup/ErrorControlChainTest.php +++ b/tests/core/startup/ErrorControlChainTest.php @@ -8,6 +8,30 @@ */ class ErrorControlChainTest_Chain extends ErrorControlChain { + protected $displayErrors = 'STDERR'; + + /** + * Modify method visibility to public for testing + * + * @return string + */ + public function getDisplayErrors() + { + // Protect manipulation of underlying php_ini values + return $this->displayErrors; + } + + /** + * Modify method visibility to public for testing + * + * @param mixed $errors + */ + public function setDisplayErrors($errors) + { + // Protect manipulation of underlying php_ini values + $this->displayErrors = $errors; + } + // Change function visibility to be testable directly public function translateMemstring($memstring) { return parent::translateMemstring($memstring); @@ -63,10 +87,7 @@ require_once '$classpath'; class ErrorControlChainTest extends SapphireTest { - protected $displayErrors = null; - function setUp() { - $this->displayErrors = (bool)ini_get('display_errors'); // Check we can run PHP at all $null = is_writeable('/dev/null') ? '/dev/null' : 'NUL'; @@ -80,50 +101,55 @@ class ErrorControlChainTest extends SapphireTest { parent::setUp(); } - public function tearDown() { - if($this->displayErrors !== null) { - ini_set('display_errors', $this->displayErrors); - $this->displayErrors = null; - } - parent::tearDown(); // TODO: Change the autogenerated stub - } - function testErrorSuppression() { // Errors disabled by default - ini_set('display_errors', false); - $chain = new ErrorControlChain(); + $chain = new ErrorControlChainTest_Chain(); + $chain->setDisplayErrors('Off'); // mocks display_errors: Off + $initialValue = null; $whenNotSuppressed = null; $whenSuppressed = null; - $chain->then(function($chain) use(&$whenNotSuppressed, &$whenSuppressed) { - $chain->setSuppression(true); - $whenSuppressed = ini_get('display_errors'); - $chain->setSuppression(false); - $whenNotSuppressed = ini_get('display_errors'); - })->execute(); + $chain->then( + function(ErrorControlChainTest_Chain $chain) + use(&$initialValue, &$whenNotSuppressed, &$whenSuppressed) { + $initialValue = $chain->getDisplayErrors(); + $chain->setSuppression(false); + $whenNotSuppressed = $chain->getDisplayErrors(); + $chain->setSuppression(true); + $whenSuppressed = $chain->getDisplayErrors(); + } + )->execute(); // Disabled errors never un-disable - $this->assertFalse((bool)$whenNotSuppressed); - $this->assertFalse((bool)$whenSuppressed); + $this->assertEquals(0, $initialValue); // Chain starts suppressed + $this->assertEquals(0, $whenSuppressed); // false value used internally when suppressed + $this->assertEquals('Off', $whenNotSuppressed); // false value set by php ini when suppression lifted + $this->assertEquals('Off', $chain->getDisplayErrors()); // Correctly restored after run // Errors enabled by default - ini_set('display_errors', true); - $chain = new ErrorControlChain(); + $chain = new ErrorControlChainTest_Chain(); + $chain->setDisplayErrors('Yes'); // non-falsey ini value + $initialValue = null; $whenNotSuppressed = null; $whenSuppressed = null; - $chain->then(function($chain) use(&$whenNotSuppressed, &$whenSuppressed) { - $chain->setSuppression(true); - $whenSuppressed = ini_get('display_errors'); - $chain->setSuppression(false); - $whenNotSuppressed = ini_get('display_errors'); - })->execute(); + $chain->then( + function(ErrorControlChainTest_Chain $chain) + use(&$initialValue, &$whenNotSuppressed, &$whenSuppressed) { + $initialValue = $chain->getDisplayErrors(); + $chain->setSuppression(true); + $whenSuppressed = $chain->getDisplayErrors(); + $chain->setSuppression(false); + $whenNotSuppressed = $chain->getDisplayErrors(); + } + )->execute(); // Errors can be suppressed an un-suppressed when initially enabled - $this->assertTrue((bool)$whenNotSuppressed); - $this->assertFalse((bool)$whenSuppressed); + $this->assertEquals(0, $initialValue); // Chain starts suppressed + $this->assertEquals(0, $whenSuppressed); // false value used internally when suppressed + $this->assertEquals('Yes', $whenNotSuppressed); // false value set by php ini when suppression lifted + $this->assertEquals('Yes', $chain->getDisplayErrors()); // Correctly restored after run // Fatal error - $chain = new ErrorControlChainTest_Chain(); list($out, $code) = $chain diff --git a/tests/forms/gridfield/GridFieldExportButtonTest.php b/tests/forms/gridfield/GridFieldExportButtonTest.php index 8629d1c16..42ef28e22 100644 --- a/tests/forms/gridfield/GridFieldExportButtonTest.php +++ b/tests/forms/gridfield/GridFieldExportButtonTest.php @@ -115,6 +115,18 @@ class GridFieldExportButtonTest extends SapphireTest { $button->generateExportFileData($this->gridField) ); } + + public function testZeroValue() { + $button = new GridFieldExportButton(); + $button->setExportColumns(array( + 'RugbyTeamNumber' => 'Rugby Team Number' + )); + + $this->assertEquals( + "\"Rugby Team Number\"\n\"2\"\n\"0\"\n", + $button->generateExportFileData($this->gridField) + ); + } } /** @@ -125,7 +137,8 @@ class GridFieldExportButtonTest_Team extends DataObject implements TestOnly { private static $db = array( 'Name' => 'Varchar', - 'City' => 'Varchar' + 'City' => 'Varchar', + 'RugbyTeamNumber' => 'Int' ); public function canView($member = null) { @@ -142,7 +155,8 @@ class GridFieldExportButtonTest_NoView extends DataObject implements TestOnly { private static $db = array( 'Name' => 'Varchar', - 'City' => 'Varchar' + 'City' => 'Varchar', + 'RugbyTeamNumber' => 'Int' ); public function canView($member = null) { diff --git a/tests/forms/gridfield/GridFieldExportButtonTest.yml b/tests/forms/gridfield/GridFieldExportButtonTest.yml index a44523e1d..27a1ae06b 100644 --- a/tests/forms/gridfield/GridFieldExportButtonTest.yml +++ b/tests/forms/gridfield/GridFieldExportButtonTest.yml @@ -2,9 +2,11 @@ GridFieldExportButtonTest_Team: test-team-1: Name: Test City: City + RugbyTeamNumber: 2 test-team-2: Name: Test2 City: City2 + RugbyTeamNumber: 0 GridFieldExportButtonTest_NoView: item1: