Commit 2d342c0f authored by alexpott's avatar alexpott

Issue #2510380 by Wim Leers, DuaelFr, mlewand, quicksketch, Reinmar, oleq:...

Issue #2510380 by Wim Leers, DuaelFr, mlewand, quicksketch, Reinmar, oleq: Images cannot be linked in CKEditor
parent c7a9c9ae
...@@ -65,14 +65,11 @@ ...@@ -65,14 +65,11 @@
} }
widgetDefinition.allowedContent = new CKEDITOR.style(allowedContentDefinition); widgetDefinition.allowedContent = new CKEDITOR.style(allowedContentDefinition);
// Override the 'link' part, to completely disable image2's link
// support: http://dev.ckeditor.com/ticket/11341.
widgetDefinition.parts.link = 'This is a nonsensical selector to disable this functionality completely';
// Override downcast(): since we only accept <img> in our upcast method, // Override downcast(): since we only accept <img> in our upcast method,
// the element is already correct. We only need to update the element's // the element is already correct. We only need to update the element's
// data-entity-uuid attribute. // data-entity-uuid attribute.
widgetDefinition.downcast = function (element) { widgetDefinition.downcast = function (element) {
element.attributes['data-entity-type'] = this.data['data-entity-type'];
element.attributes['data-entity-uuid'] = this.data['data-entity-uuid']; element.attributes['data-entity-uuid'] = this.data['data-entity-uuid'];
}; };
...@@ -176,6 +173,18 @@ ...@@ -176,6 +173,18 @@
return widget; return widget;
}; };
}; };
var originalInit = widgetDefinition.init;
widgetDefinition.init = function () {
originalInit.call(this);
// Update data.link object with attributes if the link has been
// discovered.
// @see plugins/image2/plugin.js/init() in CKEditor; this is similar.
if (this.parts.link) {
this.setData('link', CKEDITOR.plugins.link.parseLinkAttributes(editor, this.parts.link));
}
};
}); });
// Add a widget#edit listener to every instance of image2 widget in order // Add a widget#edit listener to every instance of image2 widget in order
...@@ -233,25 +242,86 @@ ...@@ -233,25 +242,86 @@
} }
}, },
// Disable image2's integration with the link/drupallink plugins: don't
// allow the widget itself to become a link. Support for that may be added
// by an text filter that adds a data- attribute specifically for that.
afterInit: function (editor) { afterInit: function (editor) {
if (editor.plugins.drupallink) { linkCommandIntegrator(editor);
var cmd = editor.getCommand('drupallink');
// Needs to be refreshed on selection changes.
cmd.contextSensitive = 1;
// Disable command and cancel event when the image widget is selected.
cmd.on('refresh', function (evt) {
var widget = editor.widgets.focused;
if (widget && widget.name === 'image') {
this.setState(CKEDITOR.TRISTATE_DISABLED);
evt.cancel();
}
});
}
} }
}); });
/**
* Integrates the drupalimage widget with the drupallink plugin.
*
* Makes images linkable.
*
* @param {CKEDITOR.editor} editor
* A CKEditor instance.
*/
function linkCommandIntegrator(editor) {
// Nothing to integrate with if the drupallink plugin is not loaded.
if (!editor.plugins.drupallink) {
return;
}
// Override default behaviour of 'drupalunlink' command.
editor.getCommand('drupalunlink').on('exec', function (evt) {
var widget = getFocusedWidget(editor);
// Override 'drupalunlink' only when link truly belongs to the widget. If
// wrapped inline widget in a link, let default unlink work.
// @see https://dev.ckeditor.com/ticket/11814
if (!widget || !widget.parts.link) {
return;
}
widget.setData('link', null);
// Selection (which is fake) may not change if unlinked image in focused
// widget, i.e. if captioned image. Let's refresh command state manually
// here.
this.refresh(editor, editor.elementPath());
evt.cancel();
});
// Override default refresh of 'drupalunlink' command.
editor.getCommand('drupalunlink').on('refresh', function (evt) {
var widget = getFocusedWidget(editor);
if (!widget) {
return;
}
// Note that widget may be wrapped in a link, which
// does not belong to that widget (#11814).
this.setState(widget.data.link || widget.wrapper.getAscendant('a') ?
CKEDITOR.TRISTATE_OFF : CKEDITOR.TRISTATE_DISABLED);
evt.cancel();
});
}
/**
* Gets the focused widget, if of the type specific for this plugin.
*
* @param {CKEDITOR.editor} editor
* A CKEditor instance.
*
* @return {?CKEDITOR.plugins.widget}
* The focused image2 widget instance, or null.
*/
function getFocusedWidget(editor) {
var widget = editor.widgets.focused;
if (widget && widget.name === 'image') {
return widget;
}
return null;
}
// Expose an API for other plugins to interact with drupalimage widgets.
CKEDITOR.plugins.drupalimage = {
getFocusedWidget: getFocusedWidget
};
})(jQuery, Drupal, CKEDITOR); })(jQuery, Drupal, CKEDITOR);
...@@ -71,10 +71,9 @@ ...@@ -71,10 +71,9 @@
// data-caption attributes. // data-caption attributes.
var originalDowncast = widgetDefinition.downcast; var originalDowncast = widgetDefinition.downcast;
widgetDefinition.downcast = function (element) { widgetDefinition.downcast = function (element) {
var img = originalDowncast.call(this, element); var img = findElementByName(element, 'img');
if (!img) { originalDowncast.call(this, img);
img = findElementByName(element, 'img');
}
var caption = this.editables.caption; var caption = this.editables.caption;
var captionHtml = caption && caption.getData(); var captionHtml = caption && caption.getData();
var attrs = img.attributes; var attrs = img.attributes;
...@@ -91,10 +90,14 @@ ...@@ -91,10 +90,14 @@
attrs['data-align'] = this.data.align; attrs['data-align'] = this.data.align;
} }
} }
attrs['data-entity-type'] = this.data['data-entity-type'];
attrs['data-entity-uuid'] = this.data['data-entity-uuid'];
return img; // If img is wrapped with a link, we want to return that link.
if (img.parent.name === 'a') {
return img.parent;
}
else {
return img;
}
}; };
// We want to upcast <img> elements to a DOM structure required by the // We want to upcast <img> elements to a DOM structure required by the
...@@ -115,6 +118,11 @@ ...@@ -115,6 +118,11 @@
element = originalUpcast.call(this, element, data); element = originalUpcast.call(this, element, data);
var attrs = element.attributes; var attrs = element.attributes;
if (element.parent.name === 'a') {
element = element.parent;
}
var retElement = element; var retElement = element;
var caption; var caption;
......
...@@ -31,6 +31,8 @@ ...@@ -31,6 +31,8 @@
modes: {wysiwyg: 1}, modes: {wysiwyg: 1},
canUndo: true, canUndo: true,
exec: function (editor) { exec: function (editor) {
var drupalImageUtils = CKEDITOR.plugins.drupalimage;
var focusedImageWidget = drupalImageUtils && drupalImageUtils.getFocusedWidget(editor);
var linkElement = getSelectedLink(editor); var linkElement = getSelectedLink(editor);
var linkDOMElement = null; var linkDOMElement = null;
...@@ -56,9 +58,30 @@ ...@@ -56,9 +58,30 @@
existingValues[attributeName] = linkElement.data('cke-saved-' + attributeName) || attribute.nodeValue; existingValues[attributeName] = linkElement.data('cke-saved-' + attributeName) || attribute.nodeValue;
} }
} }
// Or, if an image widget is focused, we're editing a link wrapping
// an image widget.
else if (focusedImageWidget && focusedImageWidget.data.link) {
var url = focusedImageWidget.data.link.url;
existingValues.href = url.protocol + url.url;
}
// Prepare a save callback to be used upon saving the dialog. // Prepare a save callback to be used upon saving the dialog.
var saveCallback = function (returnValues) { var saveCallback = function (returnValues) {
// If an image widget is focused, we're not editing an independent
// link, but we're wrapping an image widget in a link.
if (focusedImageWidget) {
var urlMatch = returnValues.attributes.href.match(urlRegex);
focusedImageWidget.setData('link', {
type: 'url',
url: {
protocol: urlMatch[1],
url: urlMatch[2]
}
});
editor.fire('saveSnapshot');
return;
}
editor.fire('saveSnapshot'); editor.fire('saveSnapshot');
// Create a new link element if needed. // Create a new link element if needed.
...@@ -256,4 +279,57 @@ ...@@ -256,4 +279,57 @@
return null; return null;
} }
var urlRegex = /^((?:http|https):\/\/)?(.*)$/;
/**
* The image2 plugin is currently tightly coupled to the link plugin: it
* calls CKEDITOR.plugins.link.parseLinkAttributes().
*
* Drupal 8's CKEditor build doesn't include the 'link' plugin. Because it
* includes its own link plugin that integrates with Drupal's dialog system.
* So, to allow images to be linked, we need to duplicate the necessary subset
* of the logic.
*
* @todo Remove once we update to CKEditor 4.5.5.
* @see https://dev.ckeditor.com/ticket/13885
*/
CKEDITOR.plugins.link = CKEDITOR.plugins.link || {
parseLinkAttributes: function (editor, element) {
var href = (element && (element.data('cke-saved-href') || element.getAttribute('href'))) || '';
var urlMatch = href.match(urlRegex);
return {
type: 'url',
url: {
protocol: urlMatch[1],
url: urlMatch[2]
}
};
},
getLinkAttributes: function (editor, data) {
var set = {};
var protocol = (data.url && typeof data.url.protocol !== 'undefined') ? data.url.protocol : 'http://';
var url = (data.url && CKEDITOR.tools.trim(data.url.url)) || '';
set['data-cke-saved-href'] = (url.indexOf('/') === 0) ? url : protocol + url;
// Browser need the "href" fro copy/paste link to work. (#6641)
if (set['data-cke-saved-href']) {
set.href = set['data-cke-saved-href'];
}
// Remove all attributes which are not currently set.
var removed = {};
for (var s in set) {
if (set.hasOwnProperty(s)) {
delete removed[s];
}
}
return {
set: set,
removed: CKEDITOR.tools.objectKeys(removed)
};
}
};
})(jQuery, Drupal, drupalSettings, CKEDITOR); })(jQuery, Drupal, drupalSettings, CKEDITOR);
...@@ -55,15 +55,17 @@ public function process($text, $langcode) { ...@@ -55,15 +55,17 @@ public function process($text, $langcode) {
// Given the updated node and caption: re-render it with a caption, but // Given the updated node and caption: re-render it with a caption, but
// bubble up the value of the class attribute of the captioned element, // bubble up the value of the class attribute of the captioned element,
// this allows it to collaborate with e.g. the filter_align filter. // this allows it to collaborate with e.g. the filter_align filter.
$tag = $node->tagName;
$classes = $node->getAttribute('class'); $classes = $node->getAttribute('class');
$node->removeAttribute('class'); $node->removeAttribute('class');
$node = ($node->parentNode->tagName === 'a') ? $node->parentNode : $node;
$filter_caption = array( $filter_caption = array(
'#theme' => 'filter_caption', '#theme' => 'filter_caption',
// We pass the unsanitized string because this is a text format // We pass the unsanitized string because this is a text format
// filter, and after filtering, we always assume the output is safe. // filter, and after filtering, we always assume the output is safe.
// @see \Drupal\filter\Element\ProcessedText::preRenderText() // @see \Drupal\filter\Element\ProcessedText::preRenderText()
'#node' => FilteredMarkup::create($node->C14N()), '#node' => FilteredMarkup::create($node->C14N()),
'#tag' => $node->tagName, '#tag' => $tag,
'#caption' => $caption, '#caption' => $caption,
'#classes' => $classes, '#classes' => $classes,
); );
...@@ -78,7 +80,7 @@ public function process($text, $langcode) { ...@@ -78,7 +80,7 @@ public function process($text, $langcode) {
// Import the updated node from the new DOMDocument into the original // Import the updated node from the new DOMDocument into the original
// one, importing also the child nodes of the updated node. // one, importing also the child nodes of the updated node.
$updated_node = $dom->importNode($updated_node, TRUE); $updated_node = $dom->importNode($updated_node, TRUE);
// Finally, replace the original image node with the new image node! // Finally, replace the original node with the new node.
$node->parentNode->replaceChild($updated_node, $node); $node->parentNode->replaceChild($updated_node, $node);
} }
......
...@@ -184,6 +184,13 @@ function testCaptionFilter() { ...@@ -184,6 +184,13 @@ function testCaptionFilter() {
$this->assertIdentical($expected, $output->getProcessedText()); $this->assertIdentical($expected, $output->getProcessedText());
$this->assertIdentical($attached_library, $output->getAttachments()); $this->assertIdentical($attached_library, $output->getAttachments());
// Ensure the caption filter works for linked images.
$input = '<a href="http://example.com/llamas/are/awesome/but/kittens/are/cool/too"><img src="llama.jpg" data-caption="Loquacious llama!" /></a>';
$expected = '<figure role="group"><a href="http://example.com/llamas/are/awesome/but/kittens/are/cool/too"><img src="llama.jpg" /></a>' . "\n" . '<figcaption>Loquacious llama!</figcaption></figure>';
$output = $test($input);
$this->assertIdentical($expected, $output->getProcessedText());
$this->assertIdentical($attached_library, $output->getAttachments());
// So far we've tested that the caption filter works correctly. But we also // So far we've tested that the caption filter works correctly. But we also
// want to make sure that it works well in tandem with the "Limit allowed // want to make sure that it works well in tandem with the "Limit allowed
// HTML tags" filter, which it is typically used with. // HTML tags" filter, which it is typically used with.
...@@ -301,6 +308,13 @@ function testAlignAndCaptionFilters() { ...@@ -301,6 +308,13 @@ function testAlignAndCaptionFilters() {
$output = $test($input); $output = $test($input);
$this->assertIdentical($expected, $output->getProcessedText()); $this->assertIdentical($expected, $output->getProcessedText());
$this->assertIdentical($attached_library, $output->getAttachments()); $this->assertIdentical($attached_library, $output->getAttachments());
// Ensure both filters together work for linked images.
$input = '<a href="http://example.com/llamas/are/awesome/but/kittens/are/cool/too"><img src="llama.jpg" data-caption="Loquacious llama!" data-align="center" /></a>';
$expected = '<figure role="group" class="align-center"><a href="http://example.com/llamas/are/awesome/but/kittens/are/cool/too"><img src="llama.jpg" /></a>' . "\n" . '<figcaption>Loquacious llama!</figcaption></figure>';
$output = $test($input);
$this->assertIdentical($expected, $output->getProcessedText());
$this->assertIdentical($attached_library, $output->getAttachments());
} }
/** /**
......
...@@ -170,17 +170,20 @@ function simpletest_run_tests($test_list) { ...@@ -170,17 +170,20 @@ function simpletest_run_tests($test_list) {
* @param $unescaped_test_classnames * @param $unescaped_test_classnames
* An array of test class names, including full namespaces, to be passed as * An array of test class names, including full namespaces, to be passed as
* a regular expression to PHPUnit's --filter option. * a regular expression to PHPUnit's --filter option.
* @param int $status
* (optional) The exit status code of the PHPUnit process will be assigned to
* this variable.
* *
* @return array * @return array
* The parsed results of PHPUnit's JUnit XML output, in the format of * The parsed results of PHPUnit's JUnit XML output, in the format of
* {simpletest}'s schema. * {simpletest}'s schema.
*/ */
function simpletest_run_phpunit_tests($test_id, array $unescaped_test_classnames) { function simpletest_run_phpunit_tests($test_id, array $unescaped_test_classnames, &$status = NULL) {
$phpunit_file = simpletest_phpunit_xml_filepath($test_id); $phpunit_file = simpletest_phpunit_xml_filepath($test_id);
$ret = simpletest_phpunit_run_command($unescaped_test_classnames, $phpunit_file); simpletest_phpunit_run_command($unescaped_test_classnames, $phpunit_file, $status);
// A return value of 0 = passed test, 1 = failed test, > 1 indicates segfault // A $status of 0 = passed test, 1 = failed test, > 1 indicates segfault
// timeout, or other type of failure. // timeout, or other type of failure.
if ($ret > 1) { if ($status > 1) {
// Something broke during the execution of phpunit. // Something broke during the execution of phpunit.
// Return an error record of all failed classes. // Return an error record of all failed classes.
$rows[] = [ $rows[] = [
...@@ -251,11 +254,14 @@ function simpletest_phpunit_configuration_filepath() { ...@@ -251,11 +254,14 @@ function simpletest_phpunit_configuration_filepath() {
* a regular expression to PHPUnit's --filter option. * a regular expression to PHPUnit's --filter option.
* @param string $phpunit_file * @param string $phpunit_file
* A filepath to use for PHPUnit's --log-junit option. * A filepath to use for PHPUnit's --log-junit option.
* @param int $status
* (optional) The exit status code of the PHPUnit process will be assigned to
* this variable.
* *
* @return string * @return string
* The results as returned by exec(). * The results as returned by exec().
*/ */
function simpletest_phpunit_run_command(array $unescaped_test_classnames, $phpunit_file) { function simpletest_phpunit_run_command(array $unescaped_test_classnames, $phpunit_file, &$status = NULL) {
// Setup an environment variable containing the database connection so that // Setup an environment variable containing the database connection so that
// functional tests can connect to the database. // functional tests can connect to the database.
putenv('SIMPLETEST_DB=' . Database::getConnectionInfoAsUrl()); putenv('SIMPLETEST_DB=' . Database::getConnectionInfoAsUrl());
...@@ -292,7 +298,8 @@ function simpletest_phpunit_run_command(array $unescaped_test_classnames, $phpun ...@@ -292,7 +298,8 @@ function simpletest_phpunit_run_command(array $unescaped_test_classnames, $phpun
// exec in a subshell so that the environment is isolated when running tests // exec in a subshell so that the environment is isolated when running tests
// via the simpletest UI. // via the simpletest UI.
exec(join($command, " "), $output, $ret); $ret = exec(join($command, " "), $output, $status);
chdir($old_cwd); chdir($old_cwd);
putenv('SIMPLETEST_DB='); putenv('SIMPLETEST_DB=');
return $ret; return $ret;
......
This diff is collapsed.
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment