From eef67b6f8761ac947dc473defd46e2c6c4dbc083 Mon Sep 17 00:00:00 2001 From: Mateusz Uzdowski Date: Tue, 10 Jul 2012 16:30:13 +1200 Subject: [PATCH 1/4] API Clean up the return values for getCurrentLink Returning a link type "internal" in the situation when no link has been detected is confusing and makes it hard to know downstream if the link was detected or not. Switched that to null. Also added target option to file downloads, as we don't currently have a mechanism to default this field to "yes" for files. --- javascript/HtmlEditorField.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/javascript/HtmlEditorField.js b/javascript/HtmlEditorField.js index 8bac10c0e..309aa1f32 100644 --- a/javascript/HtmlEditorField.js +++ b/javascript/HtmlEditorField.js @@ -631,8 +631,9 @@ ss.editorWrappers['default'] = ss.editorWrappers.tinyMCE; } }, /** - * Return information about the currently selected link, suitable for population of the link - * form. + * Return information about the currently selected link, suitable for population of the link form. + * + * Returns null if no link was currently selected. */ getCurrentLink: function() { var selectedEl = this.getSelection(), @@ -682,7 +683,8 @@ ss.editorWrappers['default'] = ss.editorWrappers.tinyMCE; return { LinkType: 'file', file: RegExp.$1, - Description: title + Description: title, + TargetBlank: target ? true : false }; } else if(href.match(/^#(.*)$/)) { return { @@ -707,9 +709,8 @@ ss.editorWrappers['default'] = ss.editorWrappers.tinyMCE; TargetBlank: target ? true : false }; } else { - return { - LinkType: 'internal' - }; + // No link/invalid link selected. + return null; } } }); From 29a039929b0505bd560a75048dee1aea550c74df Mon Sep 17 00:00:00 2001 From: Mateusz Uzdowski Date: Tue, 10 Jul 2012 16:32:39 +1200 Subject: [PATCH 2/4] Refactor the linking functionality for the TinyMCE popup. Functionality that affects the values in the form better fits in updateFromEditor function, where we expect the form to be modified. Redraw should only affect visibility parameters. Also added a more robust reset code, so we can always expect to get at least a clean form, and re-added missing "target" checkbox. --- javascript/HtmlEditorField.js | 36 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/javascript/HtmlEditorField.js b/javascript/HtmlEditorField.js index 309aa1f32..9a271a47c 100644 --- a/javascript/HtmlEditorField.js +++ b/javascript/HtmlEditorField.js @@ -468,7 +468,6 @@ ss.editorWrappers['default'] = ss.editorWrappers.tinyMCE; * which are toggled through a type dropdown. Variations share fields, so there's only one "title" field in the form. */ $('form.htmleditorfield-linkform').entwine({ - // TODO Entwine doesn't respect submits triggered by ENTER key onsubmit: function(e) { this.insertLink(); @@ -477,36 +476,27 @@ ss.editorWrappers['default'] = ss.editorWrappers.tinyMCE; }, resetFields: function() { this._super(); - this.find('fieldset :input:not(:radio)').val('').change(); + + // Reset the form using a native call. This will also correctly reset checkboxes and radio buttons. + this[0].reset(); }, - redraw: function(setDefaults) { + redraw: function() { this._super(); var linkType = this.find(':input[name=LinkType]:checked').val(), list = ['internal', 'external', 'file', 'email']; - // If we haven't selected an existing link, then just make sure we default to "internal" for the link type. - if(!linkType) { - this.find(':input[name=LinkType]').val(['internal']); - linkType = 'internal'; - } - this.addAnchorSelector(); - // Toggle field visibility and state based on type selection + // Toggle field visibility depending on the link type. this.find('div.content .field').hide(); this.find('.field#LinkType').show(); this.find('.field#' + linkType).show(); if(linkType == 'internal' || linkType == 'anchor') this.find('.field#Anchor').show(); + if(linkType !== 'email') this.find('.field#TargetBlank').show(); if(linkType == 'anchor') { this.find('.field#AnchorSelector').show(); this.find('.field#AnchorRefresh').show(); } - - this.find(':input[name=TargetBlank]').attr('disabled', (linkType == 'email')); - - if(typeof setDefaults == 'undefined' || setDefaults) { - this.find(':input[name=TargetBlank]').attr('checked', (linkType == 'file')); - } }, insertLink: function() { var href, target = null, anchor = this.find(':input[name=Anchor]').val(); @@ -614,16 +604,24 @@ ss.editorWrappers['default'] = ss.editorWrappers.tinyMCE; selector.append($('')); } }, + /** + * Updates the state of the dialog inputs to match the editor selection. + * If selection does not contain a link, resets the fields. + */ updateFromEditor: function() { var htmlTagPattern = /<\S[^><]*>/g, fieldName, data = this.getCurrentLink(); - + if(data) { for(fieldName in data) { var el = this.find(':input[name=' + fieldName + ']'), selected = data[fieldName]; // Remove html tags in the selected text that occurs on IE browsers if(typeof(selected) == 'string') selected = selected.replace(htmlTagPattern, ''); - if(el.is(':radio')) { - el.val([selected]).change(); // setting as an arry due to jQuery quirks + + // Set values and invoke the triggers (e.g. for TreeDropdownField). + if(el.is(':checkbox')) { + el.prop('checked', selected).change(); + } else if(el.is(':radio')) { + el.val([selected]).change(); } else { el.val(selected).change(); } From 7c41ff22ab227c27f25ed8f796872a50bfb71d83 Mon Sep 17 00:00:00 2001 From: Mateusz Uzdowski Date: Thu, 12 Jul 2012 14:11:47 +1200 Subject: [PATCH 3/4] BUG Change all shortcodes into the new, comma separated, format. Space delimiter is often confused by browsers, and encoded as %20 which breaks the shortcode system. Change to comma delimitation has already been implemented, this is a followup cleanup. Ref http://open.silverstripe.org/ticket/7337 --- forms/HtmlEditorField.php | 6 +++--- javascript/HtmlEditorField.js | 2 +- parsers/ShortcodeParser.php | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/forms/HtmlEditorField.php b/forms/HtmlEditorField.php index 47b0150da..7a71fd0a5 100644 --- a/forms/HtmlEditorField.php +++ b/forms/HtmlEditorField.php @@ -63,14 +63,14 @@ class HtmlEditorField extends TextareaField { if($links = $value->getElementsByTagName('a')) foreach($links as $link) { $matches = array(); - if(preg_match('/\[sitetree_link id=([0-9]+)\]/i', $link->getAttribute('href'), $matches)) { + if(preg_match('/\[sitetree_link(?:\s*|%20|,)?id=([0-9]+)\]/i', $link->getAttribute('href'), $matches)) { if(!DataObject::get_by_id('SiteTree', $matches[1])) { $class = $link->getAttribute('class'); $link->setAttribute('class', ($class ? "$class ss-broken" : 'ss-broken')); } } - if(preg_match('/\[file_link id=([0-9]+)\]/i', $link->getAttribute('href'), $matches)) { + if(preg_match('/\[file_link(?:\s*|%20|,)?id=([0-9]+)\]/i', $link->getAttribute('href'), $matches)) { if(!DataObject::get_by_id('File', $matches[1])) { $class = $link->getAttribute('class'); $link->setAttribute('class', ($class ? "$class ss-broken" : 'ss-broken')); @@ -114,7 +114,7 @@ class HtmlEditorField extends TextareaField { $href = Director::makeRelative($link->getAttribute('href')); if($href) { - if(preg_match('/\[sitetree_link id=([0-9]+)\]/i', $href, $matches)) { + if(preg_match('/\[sitetree_link,id=([0-9]+)\]/i', $href, $matches)) { $ID = $matches[1]; // clear out any broken link classes diff --git a/javascript/HtmlEditorField.js b/javascript/HtmlEditorField.js index 9a271a47c..7f21670d6 100644 --- a/javascript/HtmlEditorField.js +++ b/javascript/HtmlEditorField.js @@ -691,7 +691,7 @@ ss.editorWrappers['default'] = ss.editorWrappers.tinyMCE; Description: title, TargetBlank: target ? true : false }; - } else if(href.match(/^\[sitetree_link\s*(?:%20)?id=([0-9]+)\]?(#.*)?$/)) { + } else if(href.match(/^\[sitetree_link(?:\s*|%20|,)?id=([0-9]+)\]?(#.*)?$/i)) { return { LinkType: 'internal', internal: RegExp.$1, diff --git a/parsers/ShortcodeParser.php b/parsers/ShortcodeParser.php index b6d2feff5..56450f8f5 100644 --- a/parsers/ShortcodeParser.php +++ b/parsers/ShortcodeParser.php @@ -29,7 +29,7 @@ * * Inbuilt Shortcodes * - * From 2.4 onwards links inserted via the CMS into a content field are in the form ''''. At runtime this is replaced by a plain link to the page with the ID in question. + * From 2.4 onwards links inserted via the CMS into a content field are in the form '''', and from 3.0 the comma is used as a separator instead ''''. At runtime this is replaced by a plain link to the page with the ID in question. * * Limitations * From c785f3c4923991b065dbda6768c68c8ea6872714 Mon Sep 17 00:00:00 2001 From: Mateusz Uzdowski Date: Thu, 12 Jul 2012 14:46:23 +1200 Subject: [PATCH 4/4] BUG Adjust the tree construction and triggers to work with IE. After Hamish's suggestion. Entwine onchange would not get executed in IE8 at all, which would have the effect of the displyed dropdown selection not being set. Change to onadd also mandates the changes to onadd on other parts of the tree component - otherwise the change event can trigger before the tree elements are added to the DOM. --- javascript/TreeDropdownField.js | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/javascript/TreeDropdownField.js b/javascript/TreeDropdownField.js index ab9ce3c35..224733f85 100644 --- a/javascript/TreeDropdownField.js +++ b/javascript/TreeDropdownField.js @@ -29,7 +29,7 @@ * @todo Expand title height to fit all elements */ $('.TreeDropdownField').entwine({ - onmatch: function() { + onadd: function() { this.append( '' + '
' + @@ -43,9 +43,6 @@ this.getPanel().hide(); this._super(); }, - onunmatch: function() { - this._super(); - }, getPanel: function() { return this.find('.treedropdownfield-panel'); }, @@ -260,7 +257,7 @@ }); $('.TreeDropdownField.searchable').entwine({ - onmatch: function() { + onadd: function() { this._super(); var title = this.data('title'); @@ -270,9 +267,6 @@ this.setTitle(title ? title : strings.searchFieldTitle); }, - onunmatch: function() { - this._super(); - }, setTitle: function(title) { if(!title && title !== '') title = strings.fieldTitle; @@ -372,8 +366,13 @@ }); $('.TreeDropdownField input[type=hidden]').entwine({ - onchange: function() { - this.getField().updateTitle(); + onadd: function() { + this.bind('change.TreeDropdownField', function() { + $(this).getField().updateTitle(); + }); + }, + onremove: function() { + this.unbind('.TreeDropdownField'); } }); });