diff --git a/lib/behat/form_field/behat_form_editor.php b/lib/behat/form_field/behat_form_editor.php index 3d5df09af66..96e8c0a92ae 100644 --- a/lib/behat/form_field/behat_form_editor.php +++ b/lib/behat/form_field/behat_form_editor.php @@ -51,6 +51,8 @@ class behat_form_editor extends behat_form_textarea { $editorid = $this->field->getAttribute('id'); if ($this->running_javascript()) { $value = addslashes($value); + // This will be transported in JSON, which doesn't allow newlines in strings, so we must escape them. + $value = str_replace("\n", "\\n", $value); $js = ' (function() { var editor = Y.one(document.getElementById("'.$editorid.'editable")); diff --git a/lib/editor/atto/tests/behat/clean.feature b/lib/editor/atto/tests/behat/clean.feature new file mode 100644 index 00000000000..aea3fee77bd --- /dev/null +++ b/lib/editor/atto/tests/behat/clean.feature @@ -0,0 +1,169 @@ +@editor @editor_atto @atto @editor_moodleform +Feature: Atto HTML cleanup. + In order to test html cleaning functionality, I write in a HTML atto text field. + + @javascript + Scenario: Extra UL close and orphan LI items + Given I log in as "admin" + When I open my profile in edit mode + And I click on "Show more buttons" "button" + And I click on "HTML" "button" + And I set the field "Description" to multiline: + """ +
Before
+After
+After 2
+ """ + And I click on "HTML" "button" + Then the field "Description" matches multiline: + """ +Before
+After
+After 2
+ """ + + @javascript + Scenario: Random close LI tag, extra LI open tag, missing OL tag + Given I log in as "admin" + When I open my profile in edit mode + And I click on "Show more buttons" "button" + And I click on "HTML" "button" + And I set the field "Description" to multiline: + """ +Before
After
+ """ + And I click on "HTML" "button" + Then the field "Description" matches multiline: + """ +Before
+After
+ """ + + @javascript + Scenario: Missing opening LI tags, missing closing UL tag + Given I log in as "admin" + When I open my profile in edit mode + And I click on "Show more buttons" "button" + And I click on "HTML" "button" + And I set the field "Description" to multiline: + """ +After
+ """ + And I click on "HTML" "button" + Then the field "Description" matches multiline: + """ +After
+ """ diff --git a/lib/editor/atto/yui/build/moodle-editor_atto-editor/moodle-editor_atto-editor-debug.js b/lib/editor/atto/yui/build/moodle-editor_atto-editor/moodle-editor_atto-editor-debug.js index 8691eccd87f..6af6477e770 100644 --- a/lib/editor/atto/yui/build/moodle-editor_atto-editor/moodle-editor_atto-editor-debug.js +++ b/lib/editor/atto/yui/build/moodle-editor_atto-editor/moodle-editor_atto-editor-debug.js @@ -718,7 +718,7 @@ EditorTextArea.prototype = { this.editor.setHTML(''); // Copy cleaned HTML to editable div. - this.editor.append(this._cleanHTML(this.textarea.get('value'))); + this.editor.append(this._cleanHTML(this.textarea.get('value'), true)); // Insert a paragraph in the empty contenteditable div. if (this.editor.getHTML() === '') { @@ -1393,9 +1393,10 @@ EditorClean.prototype = { * @method _cleanHTML * @private * @param {String} content The content to clean + * @param {Boolean} deepClean If true, do a more in depth (and resource intensive) cleaning of the HTML. * @return {String} The cleaned HTML */ - _cleanHTML: function(content) { + _cleanHTML: function(content, deepClean) { // Removing limited things that can break the page or a disallowed, like unclosed comments, style blocks, etc. var rules = [ @@ -1412,7 +1413,13 @@ EditorClean.prototype = { {regex: /<\/?(?:title|meta|style|st\d|head\b|font|html|body|link)[^>]*?>/gi, replace: ""} ]; - return this._filterContentWithRules(content, rules); + content = this._filterContentWithRules(content, rules); + + if (deepClean) { + content = this._cleanHTMLLists(content); + } + + return content; }, /** @@ -1443,6 +1450,8 @@ EditorClean.prototype = { pasteCleanup: function(sourceEvent) { // We only expect paste events, but we will check anyways. if (sourceEvent.type === 'paste') { + // Register the delayed paste cleanup. We will cancel it if we register the fallback cleanup. + var delayedCleanup = this.postPasteCleanupDelayed(); // The YUI event wrapper doesn't provide paste event info, so we need the underlying event. var event = sourceEvent._event; // Check if we have a valid clipboardData object in the event. @@ -1465,6 +1474,7 @@ EditorClean.prototype = { content = event.clipboardData.getData('text/html'); } catch (error) { // Something went wrong. Fallback. + delayedCleanup.cancel(); this.fallbackPasteCleanupDelayed(); return true; } @@ -1497,6 +1507,7 @@ EditorClean.prototype = { // Something went wrong. Fallback. // Due to poor cross browser clipboard compatibility, the failure to find html doesn't mean it isn't there. // Wait for the clipboard event to finish then fallback clean the entire editor. + delayedCleanup.cancel(); this.fallbackPasteCleanupDelayed(); return true; } @@ -1515,10 +1526,50 @@ EditorClean.prototype = { return true; }, + /** + * Calls postPasteCleanup on a short timer to allow the paste event handlers to complete, then deep clean the content. + * + * @method postPasteCleanupDelayed + * @return {object} + * @chainable + */ + postPasteCleanupDelayed: function() { + Y.soon(Y.bind(this.postPasteCleanup, this)); + + return this; + }, + + /** + * Do additional cleanup after the paste is complete. + * + * @method postPasteCleanup + * @return {object} + * @chainable + */ + postPasteCleanup: function() { + Y.log('Executing delayed post paste cleanup', 'debug', LOGNAME); + + // Save the current selection (cursor position). + var selection = window.rangy.saveSelection(); + + // Get, clean, and replace the content in the editable. + var content = this.editor.get('innerHTML'); + this.editor.set('innerHTML', this._cleanHTML(content, true)); + + // Update the textarea. + this.updateOriginal(); + + // Restore the selection (cursor position). + window.rangy.restoreSelection(selection); + + return this; + }, + /** * Cleanup code after a paste event if we couldn't intercept the paste content. * * @method fallbackPasteCleanup + * @return {object} * @chainable */ fallbackPasteCleanup: function() { @@ -1529,7 +1580,7 @@ EditorClean.prototype = { // Get, clean, and replace the content in the editable. var content = this.editor.get('innerHTML'); - this.editor.set('innerHTML', this._cleanPasteHTML(content)); + this.editor.set('innerHTML', this._cleanHTML(this._cleanPasteHTML(content), true)); // Update the textarea. this.updateOriginal(); @@ -1709,6 +1760,198 @@ EditorClean.prototype = { }); return holder.innerHTML; + }, + + /** + * This is a function that searches for, and attempts to correct certain issues with ul/ol html lists. + * This is needed because these lists are used heavily in page layout, and content with bad tags can + * lead to broke course pages. + * + * The theory of operation here is to linearly process the incoming content, counting the opening and closing + * of list tags, and determining when there is a mismatch. + * + * The specific issues this should be able to correct are: + * - Orphaned li elements will be wrapped in a set of ul tags. + * - li elements inside li elements. + * - An extra closing ul, or ol tag will be discarded. + * - An extra closing li tag will have an opening tag added if appropriate, or will be discarded. + * - If there is an unmatched list open tag, a matching close tag will be inserted. + * + * It does it's best to match the case of corrected tags. Even though not required by html spec, + * it seems like the safer route. + * + * A note on parent elements of li. This code assumes that li must have a ol or ul parent. + * There are two other potential other parents of li. They are menu and dir. The dir tag was deprecated in + * HTML4, and removed in HTML5. The menu tag is experimental as of this writing, and basically doesn't work + * in any browsers, even Firefox, which theoretically has limited support for it. If other parents of li + * become viable, they will need to be added to this code. + * + * @method _cleanHTMLLists + * @private + * @param {String} content The content to clean + * @return {String} The cleaned content + */ + _cleanHTMLLists: function(content) { + var output = '', + toProcess = content, + match = null, + openTags = [], + currentTag = null, + previousTag = null; + + // Use a regular expression to find the next open or close li, ul, or ol tag. + // Keep going until there are no more matching tags left. + while ((match = toProcess.match(/<(\/?)(li|ul|ol)[^>]*>/i))) { + currentTag = { + tag: match[2], + tagLowerCase: match[2].toLowerCase(), + fullTag: match[0], + isOpen: (match[1].length == 1) ? false : true + }; + + // Get the most recent open tag. + previousTag = (openTags.length) ? openTags[openTags.length - 1] : null; + + // Slice up the content based on the match and add content before the match to output. + output += toProcess.slice(0, match.index); + toProcess = toProcess.slice(match.index + match[0].length); + + // Now the full content is in output + currentTag.fullTag + toProcess. When making fixes, it is best to push the fix and + // fullTag back onto the front or toProcess, then restart the loop. This allows processing to follow the normal path + // most often. But sometimes we will need to modify output to insert or remove tags in the already complete code. + + if (currentTag.isOpen) { + // We are at the opening phase of a tag. + // We have to do special processing for list items, as they can only be children of ul and ol tags. + if (currentTag.tagLowerCase === 'li') { + if (!previousTag) { + // This means we have are opening a li, but aren't in a list. This is not allowed! + + // We are going to check for the count of open and close ol tags ahead to decide what to do. + var closeCount = (toProcess.match(/<\/(ol)[ >]/ig) || []).length; + var openCount = (toProcess.match(/<(ol)[ >]/ig) || []).length; + + if (closeCount > openCount) { + // There are more close ol's ahead than opens ahead. So open the ol and try again. + Y.log('Adding an opening ol for orphan li', 'debug', LOGNAME); + toProcess = '","
','
','
","
','
','