From 0b5d20513c3a05d3480a1d8c5713c57e2a7abf8a Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Mon, 14 Nov 2011 12:28:25 +0100 Subject: [PATCH 1/2] ENHANCEMENT Using new URLPathFilter API in SiteTree->generateURLSegment(), allowing customisation of URL filtering and transliteration --- code/model/SiteTree.php | 19 +++++++------------ tests/model/SiteTreeTest.php | 2 +- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/code/model/SiteTree.php b/code/model/SiteTree.php index 093b81d2..506d9e72 100644 --- a/code/model/SiteTree.php +++ b/code/model/SiteTree.php @@ -1385,15 +1385,10 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid if((!$this->URLSegment || $this->URLSegment == 'new-page') && $this->Title) { $this->URLSegment = $this->generateURLSegment($this->Title); } else if($this->isChanged('URLSegment')) { - // Make sure the URLSegment is valid for use in a URL - $segment = ereg_replace('[^A-Za-z0-9]+','-',$this->URLSegment); - $segment = ereg_replace('-+','-',$segment); - + $filter = Object::create('URLPathFilter'); + $this->URLSegment = $filter->filter($this->URLSegment); // If after sanitising there is no URLSegment, give it a reasonable default - if(!$segment) { - $segment = "page-$this->ID"; - } - $this->URLSegment = $segment; + if(!$this->URLSegment) $this->URLSegment = "page-$this->ID"; } // Ensure that this object has a non-conflicting URLSegment value. @@ -1583,11 +1578,11 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid * @return string Generated url segment */ function generateURLSegment($title){ - $t = Convert::raw2url($title); + $filter = Object::create('URLPathFilter'); + $t = $filter->filter($title); - if(!$t || $t == '-' || $t == '-1') { - $t = "page-$this->ID"; - } + // Fallback to generic page name if path is empty (= no valid, convertable characters) + if(!$t || $t == '-' || $t == '-1') $t = "page-$this->ID"; // Hook for extensions $this->extend('updateURLSegment', $t, $title); diff --git a/tests/model/SiteTreeTest.php b/tests/model/SiteTreeTest.php index 001477e5..ac24535e 100644 --- a/tests/model/SiteTreeTest.php +++ b/tests/model/SiteTreeTest.php @@ -101,7 +101,7 @@ class SiteTreeTest extends SapphireTest { 'staff' => 'my-staff', 'about' => 'about-us', 'staffduplicate' => 'my-staff-2', - 'product1' => '1-1-test-product', + 'product1' => '1.1-test-product', 'product2' => 'another-product', 'product3' => 'another-product-2', 'product4' => 'another-product-3', From e649082830e64a0679d1f05d8b4b951b23c177a4 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Mon, 14 Nov 2011 12:29:58 +0100 Subject: [PATCH 2/2] ENHANCEMENT Replaced client side URL filtering in CMS with ajax callbacks to new SiteTreeURLSegmentField, in order to align with extended server side logic (and avoid pre-filtering values too heavily before passing them to the server). Removed suggestions from client side user confirmation. --- code/forms/SiteTreeURLSegmentField.php | 42 +++++++++++++ code/model/SiteTree.php | 6 +- javascript/CMSMain.EditForm.js | 85 ++++++++++---------------- 3 files changed, 77 insertions(+), 56 deletions(-) create mode 100644 code/forms/SiteTreeURLSegmentField.php diff --git a/code/forms/SiteTreeURLSegmentField.php b/code/forms/SiteTreeURLSegmentField.php new file mode 100644 index 00000000..0d5a32f2 --- /dev/null +++ b/code/forms/SiteTreeURLSegmentField.php @@ -0,0 +1,42 @@ +URLSegment property, and suggest input based on the serverside rules + * defined through {@link SiteTree->generateURLSegment()} and {@link URLSegmentFilter}. + * + * Note: The actual conversion for saving the value takes place in the model layer. + */ +class SiteTreeURLSegmentField extends TextField { + + static $allowed_actions = array( + 'suggest' + ); + + function suggest($request) { + if(!$request->getVar('value')) return $this->httpError(405); + $page = $this->getPage(); + + // Same logic as SiteTree->onBeforeWrite + $page->URLSegment = $page->generateURLSegment($request->getVar('value')); + $count = 2; + while(!$page->validURLSegment()) { + $page->URLSegment = preg_replace('/-[0-9]+$/', null, $page->URLSegment) . '-' . $count; + $count++; + } + + Controller::curr()->getResponse()->addHeader('Content-Type', 'application/json'); + return Convert::raw2json(array('value' => $page->URLSegment)); + } + + /** + * @return SiteTree + */ + function getPage() { + $idField = $this->getForm()->dataFieldByName('ID'); + return ($idField && $idField->Value()) ? DataObject::get_by_id('SiteTree', $idField->Value()) : singleton('SiteTree'); + } +} \ No newline at end of file diff --git a/code/model/SiteTree.php b/code/model/SiteTree.php index 506d9e72..0f492bbe 100644 --- a/code/model/SiteTree.php +++ b/code/model/SiteTree.php @@ -1385,7 +1385,7 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid if((!$this->URLSegment || $this->URLSegment == 'new-page') && $this->Title) { $this->URLSegment = $this->generateURLSegment($this->Title); } else if($this->isChanged('URLSegment')) { - $filter = Object::create('URLPathFilter'); + $filter = Object::create('URLSegmentFilter'); $this->URLSegment = $filter->filter($this->URLSegment); // If after sanitising there is no URLSegment, give it a reasonable default if(!$this->URLSegment) $this->URLSegment = "page-$this->ID"; @@ -1578,7 +1578,7 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid * @return string Generated url segment */ function generateURLSegment($title){ - $filter = Object::create('URLPathFilter'); + $filter = Object::create('URLSegmentFilter'); $t = $filter->filter($title); // Fallback to generic page name if path is empty (= no valid, convertable characters) @@ -1828,7 +1828,7 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid new HtmlEditorField("Content", _t('SiteTree.HTMLEDITORTITLE', "Content", PR_MEDIUM, 'HTML editor title')) ), $tabMeta = new Tab('Metadata', - new TextField("URLSegment", $this->fieldLabel('URLSegment') . $urlHelper), + new SiteTreeURLSegmentField("URLSegment", $this->fieldLabel('URLSegment') . $urlHelper), new LiteralField('LinkChangeNote', self::nested_urls() && count($this->Children()) ? '

' . $this->fieldLabel('LinkChangeNote'). '

' : null ), diff --git a/javascript/CMSMain.EditForm.js b/javascript/CMSMain.EditForm.js index 2aa16f92..9c191369 100644 --- a/javascript/CMSMain.EditForm.js +++ b/javascript/CMSMain.EditForm.js @@ -21,23 +21,6 @@ * Input validation on the URLSegment field */ $('.cms-edit-form input[name=URLSegment]').entwine({ - /** - * Property: FilterRegex - * Regex - */ - FilterRegex: /[^A-Za-z0-9-]+/, - - /** - * Property: ValidationMessage - * String - */ - ValidationMessage: ss.i18n._t('CMSMAIN.URLSEGMENTVALIDATION'), - - /** - * Property: MaxLength - * Int - */ - MaxLength: 50, /** * Constructor: onmatch @@ -47,42 +30,41 @@ // intercept change event, do our own writing this.bind('change', function(e) { - if(!self.validate()) { - jQuery.noticeAdd(self.getValidationMessage()); - } - self.val(self.suggestValue(e.target.value)); - return false; + if(!self.val()) return; + + self.attr('disabled', 'disabled').parents('.field:first').addClass('loading'); + var oldVal = self.val(); + self.suggest(oldVal, function(data) { + self.removeAttr('disabled').parents('.field:first').removeClass('loading'); + var newVal = decodeURIComponent(data.value); + self.val(newVal); + + if(oldVal != newVal) { + jQuery.noticeAdd(ss.i18n._t('The URL has been changed')); + } + }); + }); this._super(); }, /** - * Function: suggestValue + * Function: suggest * * Return a value matching the criteria. * * Parameters: * (String) val - * - * Returns: - * String + * (Function) callback */ - suggestValue: function(val) { - // TODO Do we want to enforce lowercasing in URLs? - return val.substr(0, this.getMaxLength()).replace(this.getFilterRegex(), '').toLowerCase(); - }, - - /** - * Function: validate - * - * Returns: - * Boolean - */ - validate: function() { - return ( - this.val().length > this.getMaxLength() - || this.val().match(this.getFilterRegex()) + suggest: function(val, callback) { + $.get( + this.parents('form:first').attr('action') + + '/field/URLSegment/suggest/?value=' + encodeURIComponent(this.val()), + function(data) { + callback.apply(this, arguments); + } ); } }); @@ -114,24 +96,21 @@ */ updateURLSegment: function(field) { if(!field || !field.length) return; + + // TODO The new URL value is determined asynchronously, + // which means we need to come up with an alternative system + // to ask user permission to change it. // TODO language/logic coupling var isNew = this.val().indexOf("new") == 0; - var suggestion = field.entwine('ss').suggestValue(this.val()); - var confirmMessage = ss.i18n.sprintf( - ss.i18n._t( - 'UPDATEURL.CONFIRM', - 'Would you like me to change the URL to:\n\n' - + '%s/\n\nClick Ok to change the URL, ' - + 'click Cancel to leave it as:\n\n%s' - ), - suggestion, - field.val() + var confirmMessage = ss.i18n._t( + 'UPDATEURL.CONFIRMSIMPLE', + 'Do you want to update the URL from your new page title?' ); // don't ask for replacement if record is considered 'new' as defined by its title - if(isNew || (suggestion != field.val() && confirm(confirmMessage))) { - field.val(suggestion); + if(isNew || confirm(confirmMessage)) { + field.val(this.val()).trigger('change'); } } });