Issue #2362637 by yched, slashrsm, rpayanm: Editor.js attach/detach doesn't...

Issue #2362637 by yched, slashrsm, rpayanm: Editor.js attach/detach doesn't pass context all the way down

core/modules/ckeditor/src/Tests/CKEditorLoadingTest.php

@@ -116,8 +116,8 @@ function testLoading() {
     $this->assertTrue($editor_js_present, 'Text Editor JavaScript is present.');
     $this->assertTrue(count($body) === 1, 'A body field exists.');
     $this->assertTrue(count($format_selector) === 1, 'A single text format selector exists on the page.');
-    $specific_format_selector = $this->xpath('//select[contains(@class, "filter-list") and contains(@class, "editor") and @data-editor-for="edit-body-0-value"]');
-    $this->assertTrue(count($specific_format_selector) === 1, 'A single text format selector exists on the page and has the "editor" class and a "data-editor-for" attribute with the correct value.');
+    $specific_format_selector = $this->xpath('//select[contains(@class, "filter-list") and @data-editor-for="edit-body-0-value"]');
+    $this->assertTrue(count($specific_format_selector) === 1, 'A single text format selector exists on the page and has a "data-editor-for" attribute with the correct value.');
     $this->assertTrue(in_array('ckeditor/drupal.ckeditor', explode(',', $settings['ajaxPageState']['libraries'])), 'CKEditor glue library is present.');
 
     // Enable the ckeditor_test module, customize configuration. In this case,

core/modules/editor/js/editor.js

@@ -14,44 +14,45 @@
    *   A text format selector DOM element.
    *
    * @return DOM
-   *   The text area DOM element.
+   *   The text area DOM element, if it was found.
    */
   function findFieldForFormatSelector($formatSelector) {
     var field_id = $formatSelector.attr('data-editor-for');
+    // This selector will only find text areas in the top-level document. We do
+    // not support attaching editors on text areas within iframes.
     return $('#' + field_id).get(0);
   }
 
   /**
-   * Changes the text editor on the text area for the given text format selector.
+   * Changes the text editor on a text area.
    *
-   * @param jQuery $formatSelector
-   *   A text format selector DOM element.
-   * @param String activeFormatID
-   *   The currently active text format; its associated text editor will be
-   *   detached.
+   * @param DOM field
+   *   The text area DOM element.
    * @param String newFormatID
-   *   The text format we're changing to; its associated text editor will be
-   *   attached.
+   *   The text format we're changing to; the text editor for the currently
+   *   active text format will be detached, and the text editor for the new text
+   *   format will be attached.
    */
-  function changeTextEditor($formatSelector, activeFormatID, newFormatID) {
-    var originalFormatID = activeFormatID;
-    var field = findFieldForFormatSelector($formatSelector);
+  function changeTextEditor(field, newFormatID) {
+    var previousFormatID = field.getAttribute('data-editor-active-text-format');
+
     // Detach the current editor (if any) and attach a new editor.
-    if (drupalSettings.editor.formats[activeFormatID]) {
-      Drupal.editorDetach(field, drupalSettings.editor.formats[activeFormatID]);
+    if (drupalSettings.editor.formats[previousFormatID]) {
+      Drupal.editorDetach(field, drupalSettings.editor.formats[previousFormatID]);
     }
     // When no text editor is currently active, stop tracking changes.
-    else if (!drupalSettings.editor.formats[activeFormatID]) {
+    else {
       $(field).off('.editor');
     }
-    activeFormatID = newFormatID;
 
     // Attach the new text editor (if any).
-    if (drupalSettings.editor.formats[activeFormatID]) {
-      var format = drupalSettings.editor.formats[activeFormatID];
-      filterXssWhenSwitching(field, format, originalFormatID, Drupal.editorAttach);
+    if (drupalSettings.editor.formats[newFormatID]) {
+      var format = drupalSettings.editor.formats[newFormatID];
+      filterXssWhenSwitching(field, format, previousFormatID, Drupal.editorAttach);
     }
-    $formatSelector.attr('data-editor-active-text-format', newFormatID);
+
+    // Store the new active format.
+    field.setAttribute('data-editor-active-text-format', newFormatID);
   }
 
   /**
@@ -61,7 +62,8 @@
    */
   function onTextFormatChange(event) {
     var $select = $(event.target);
-    var activeFormatID = $select.attr('data-editor-active-text-format');
+    var field = event.data.field;
+    var activeFormatID = field.getAttribute('data-editor-active-text-format');
     var newFormatID = $select.val();
 
     // Prevent double-attaching if the change event is triggered manually.
@@ -75,7 +77,7 @@
     // markup to be stripped away.
     var supportContentFiltering = drupalSettings.editor.formats[newFormatID] && drupalSettings.editor.formats[newFormatID].editorSupportsContentFiltering;
     // If there is no content yet, it's always safe to change the text format.
-    var hasContent = findFieldForFormatSelector($select).value !== '';
+    var hasContent = field.value !== '';
     if (hasContent && supportContentFiltering) {
       var message = Drupal.t('Changing the text format to %text_format will permanently remove content that is not allowed in that text format.<br><br>Save your changes before switching the text format to avoid losing data.', {
         '%text_format': $select.find('option:selected').text()
@@ -89,7 +91,7 @@
             text: Drupal.t('Continue'),
             'class': 'button button--primary',
             click: function () {
-              changeTextEditor($select, activeFormatID, newFormatID);
+              changeTextEditor(field, newFormatID);
               confirmationDialog.close();
             }
           },
@@ -121,7 +123,7 @@
       confirmationDialog.showModal();
     }
     else {
-      changeTextEditor($select, activeFormatID, newFormatID);
+      changeTextEditor(field, newFormatID);
     }
   }
 
@@ -140,12 +142,19 @@
         return;
       }
 
-      $(context).find('.editor').once('editor').each(function () {
+      $(context).find('[data-editor-for]').once('editor').each(function () {
         var $this = $(this);
-        var activeFormatID = $this.val();
-        $this.attr('data-editor-active-text-format', activeFormatID);
         var field = findFieldForFormatSelector($this);
 
+        // Opt-out if no supported text area was found.
+        if (!field) {
+          return;
+        }
+
+        // Store the current active format.
+        var activeFormatID = $this.val();
+        field.setAttribute('data-editor-active-text-format', activeFormatID);
+
         // Directly attach this text editor, if the text format is enabled.
         if (settings.editor.formats[activeFormatID]) {
           // XSS protection for the current text format/editor is performed on the
@@ -165,7 +174,7 @@
 
         // Attach onChange handler to text format selector element.
         if ($this.is('select')) {
-          $this.on('change.editorAttach', onTextFormatChange);
+          $this.on('change.editorAttach', {field: field}, onTextFormatChange);
         }
         // Detach any editor when the containing form is submitted.
         $this.parents('form').on('submit', function (event) {
@@ -188,17 +197,17 @@
       if (trigger === 'serialize') {
         // Removing the editor-processed class guarantees that the editor will
         // be reattached. Only do this if we're planning to destroy the editor.
-        editors = $(context).find('.editor').findOnce('editor');
+        editors = $(context).find('[data-editor-for]').findOnce('editor');
       }
       else {
-        editors = $(context).find('.editor').removeOnce('editor');
+        editors = $(context).find('[data-editor-for]').removeOnce('editor');
       }
 
       editors.each(function () {
         var $this = $(this);
         var activeFormatID = $this.val();
         var field = findFieldForFormatSelector($this);
-        if (activeFormatID in settings.editor.formats) {
+        if (field && activeFormatID in settings.editor.formats) {
           Drupal.editorDetach(field, settings.editor.formats[activeFormatID], trigger);
         }
       });

core/modules/editor/src/Element.php

@@ -73,7 +73,6 @@ function preRenderTextFormat(array $element) {
         '#name' => $element['format']['format']['#name'],
         '#value' => $format_id,
         '#attributes' => array(
-          'class' => array('editor'),
           'data-editor-for' => $field_id,
         ),
       );

core/modules/editor/src/Tests/EditorLoadingTest.php

@@ -156,8 +156,8 @@ public function testLoading() {
     $this->assertTrue($editor_js_present, 'Text Editor JavaScript is present.');
     $this->assertTrue(count($body) === 1, 'A body field exists.');
     $this->assertTrue(count($format_selector) === 1, 'A single text format selector exists on the page.');
-    $specific_format_selector = $this->xpath('//select[contains(@class, "filter-list") and contains(@class, "editor") and @data-editor-for="edit-body-0-value"]');
-    $this->assertTrue(count($specific_format_selector) === 1, 'A single text format selector exists on the page and has the "editor" class and a "data-editor-for" attribute with the correct value.');
+    $specific_format_selector = $this->xpath('//select[contains(@class, "filter-list") and @data-editor-for="edit-body-0-value"]');
+    $this->assertTrue(count($specific_format_selector) === 1, 'A single text format selector exists on the page and has a "data-editor-for" attribute with the correct value.');
 
     // Load the editor image dialog form and make sure it does not fatal.
     $this->drupalGet('editor/dialog/image/full_html');
@@ -190,8 +190,8 @@ public function testLoading() {
     $this->assertTrue($editor_js_present, 'Text Editor JavaScript is present.');
     $this->assertTrue(count($body) === 1, 'A body field exists.');
     $this->assertTrue(count($format_selector) === 0, 'No text format selector exists on the page.');
-    $hidden_input = $this->xpath('//input[@type="hidden" and @value="plain_text" and contains(@class, "editor") and @data-editor-for="edit-body-0-value"]');
-    $this->assertTrue(count($hidden_input) === 1, 'A single text format hidden input exists on the page and has the "editor" class and a "data-editor-for" attribute with the correct value.');
+    $hidden_input = $this->xpath('//input[@type="hidden" and @value="plain_text" and @data-editor-for="edit-body-0-value"]');
+    $this->assertTrue(count($hidden_input) === 1, 'A single text format hidden input exists on the page and has a "data-editor-for" attribute with the correct value.');
 
     // Create an "article" node that uses the full_html text format, then try
     // to let the untrusted user edit it.