Issue #2384545 by Wim Leers, jbrown: $element['#ajax']['callback'] is broken,...

Issue #2384545 by Wim Leers, jbrown: $element['#ajax']['callback'] is broken, hence breaking e.g. inserting images in CKEditor

core/lib/Drupal/Core/Render/Element/RenderElement.php

@@ -137,9 +137,8 @@ public static function processAjaxForm(&$element, FormStateInterface $form_state
   /**
    * Adds Ajax information about an element to communicate with JavaScript.
    *
-   * If #ajax['url'] is set on an element, this additional JavaScript is added
-   * to the page header to attach the Ajax behaviors. See ajax.js for more
-   * information.
+   * If #ajax is set on an element, this additional JavaScript is added to the
+   * page header to attach the Ajax behaviors. See ajax.js for more information.
    *
    * @param array $element
    *   An associative array containing the properties of the element.
@@ -147,6 +146,7 @@ public static function processAjaxForm(&$element, FormStateInterface $form_state
    *   - #ajax['event']
    *   - #ajax['prevent']
    *   - #ajax['url']
+   *   - #ajax['callback']
    *   - #ajax['options']
    *   - #ajax['wrapper']
    *   - #ajax['parameters']
@@ -227,18 +227,18 @@ public static function preRenderAjaxForm($element) {
 
       $settings = $element['#ajax'];
 
-      // Assign default settings. When 'path' is set to NULL, ajax.js submits the
+      // Assign default settings. When 'url' is set to NULL, ajax.js submits the
       // Ajax request to the same URL as the form or link destination is for
       // someone with JavaScript disabled. This is generally preferred as a way to
       // ensure consistent server processing for js and no-js users, and Drupal's
       // content negotiation takes care of formatting the response appropriately.
-      // However, 'path' and 'options' may be set when wanting server processing
+      // However, 'url' and 'options' may be set when wanting server processing
       // to be substantially different for a JavaScript triggered submission.
       // One such substantial difference is form elements that use
       // #ajax['callback'] for determining which part of the form needs
       // re-rendering. For that, we have a special 'system/ajax' route.
       $settings += array(
-        'path' => isset($settings['callback']) ? 'system/ajax' : NULL,
+        'url' => isset($settings['callback']) ? Url::fromRoute('system.ajax') : NULL,
         'options' => array(),
         'accepts' => 'application/vnd.drupal-ajax'
       );
@@ -248,7 +248,7 @@ public static function preRenderAjaxForm($element) {
         $settings['method'] = 'replaceWith';
       }
 
-      // Change path to URL.
+      // Convert \Drupal\Core\Url object to string.
       if (isset($settings['url']) && $settings['url'] instanceof Url) {
         $settings['url'] = $settings['url']->setOptions($settings['options'])->toString();
       }

core/modules/system/src/Tests/Ajax/DialogTest.php

@@ -7,6 +7,8 @@
 
 namespace Drupal\system\Tests\Ajax;
 
+use Drupal\Core\Url;
+
 /**
  * Performs tests on opening and manipulating dialogs via AJAX commands.
  *
@@ -154,6 +156,19 @@ public function testDialog() {
 
     // Emulate going to the JS version of the form and check the JSON response.
     $ajax_result = $this->drupalGetAJAX('ajax-test/dialog-form', array(), array('Accept: application/vnd.drupal-modal'));
+    $expected_ajax_settings = [
+      'edit-preview' => [
+        'callback' => '::preview',
+        'event' => 'click',
+        'url' => Url::fromRoute('system.ajax')->toString(),
+        'accepts' => 'application/vnd.drupal-ajax',
+        'submit' => [
+          '_triggering_element_name' => 'op',
+          '_triggering_element_value' => 'Preview',
+        ],
+      ],
+    ];
+    $this->assertEqual($expected_ajax_settings, $ajax_result[0]['settings']['ajax']);
     $this->drupalSetContent($ajax_result[1]['data']);
     // Remove the data, the form build id and token will never match.
     unset($ajax_result[1]['data']);

core/modules/system/tests/modules/ajax_test/src/Form/AjaxTestForm.php

@@ -7,13 +7,13 @@
 
 namespace Drupal\ajax_test\Form;
 
-use Drupal\Core\Form\FormInterface;
+use Drupal\Core\Form\FormBase;
 use Drupal\Core\Form\FormStateInterface;
 
 /**
  * Dummy form for testing DialogRenderer with _form routes.
  */
-class AjaxTestForm implements FormInterface {
+class AjaxTestForm extends FormBase {
 
   /**
    * {@inheritdoc}
@@ -30,14 +30,34 @@ public function buildForm(array $form, FormStateInterface $form_state) {
     $form['#action'] = \Drupal::url('ajax_test.dialog');
 
     $form['description'] = array(
-      '#markup' => '<p>' . t("Ajax Form contents description.") . '</p>',
+      '#markup' => '<p>' . $this->t("Ajax Form contents description.") . '</p>',
     );
 
-    $form['submit'] = array(
+    $form['actions'] = array(
+      '#type' => 'actions',
+    );
+    $form['actions']['submit'] = array(
       '#type' => 'submit',
-      '#value' => t('Do it'),
+      '#value' => $this->t('Do it'),
+    );
+    $form['actions']['preview'] = array(
+      '#type' => 'submit',
+      '#value' => $this->t('Preview'),
+      // No regular submit-handler. This form only works via JavaScript.
+      '#submit' => array(),
+      '#ajax' => array(
+        // This means the ::preview() method on this class would be invoked in
+        // case of a click event. However, since Drupal core's test runner only
+        // is able to execute PHP, not JS, there is no point in actually
+        // implementing this method, because we can never let it be called from
+        // JS; we'd have to manually call it from PHP, at which point we would
+        // not actually be testing it.
+        // Therefore we consciously choose to not implement this method, because
+        // we cannot meaningfully test it anyway.
+        'callback' => '::preview',
+        'event' => 'click',
+      ),
     );
-
     return $form;
   }