Allow Dialog Controller to work with form/entity form routes

+++ b/core/lib/Drupal/Core/ContentNegotiation.phpundefined
@@ -40,6 +40,7 @@ public function getContentType(Request $request) {
+      error_log($mime_type);

debug?

Form #1842036-66: [META] Convert all confirm forms to use modal dialog while working on the issue I think we should improve DX experience for simple links and buttons. @quicksketch has added some improvements in #1879120-57: Use Drupal-specific image and link plugins — use core dialogs rather than CKEditor dialogs, containing alterable Drupal forms related to ckeditor.

+++ b/core/lib/Drupal/Core/Controller/DialogController.phpundefined
@@ -54,12 +64,34 @@ protected function forward(Request $request, $content) {
+    $request->headers->remove('Accept');
+    // Remove the X-Requested-With header so the subrequest is not mistaken for
+    // an ajax request.
+    $request->headers->remove('X-Requested-With');
+
+    // Re-enhance the routes with the help of the dynamic router.
+    $defaults = $attributes->all();
+    unset($defaults['_controller']);
+    // Create a duplicate request for forwarding and enhancing. We can't use the
+    // existing request as the protected acceptableContentTypes property has
+    // been set already at this stage and cannot be flushed.
+    $sub_request = $request->duplicate($request->query->all(), NULL, $defaults);
+    foreach ($this->dynamicRouter->getRouteEnhancers() as $enhancer) {
+      // Allow enhancers to run-again, other than the ParamConverterManager
+      // which we exclude because of a) they cannot be run again and b) it would
+      // be poor performance to do so.
+      if (get_class($enhancer) !== 'Drupal\\Core\\ParamConverter\\ParamConverterManager') {
+        $defaults = $enhancer->enhance($defaults, $sub_request);
+      }
+    }
 
-    return $this->httpKernel->forward($content, $attributes->all(), $request->query->all());
+    // Set the controller on the sub-request based on the enhancers.
+    $sub_request->attributes->set('_controller', $defaults['_controller']);
+    return $this->httpKernel->handle($sub_request, HttpKernelInterface::SUB_REQUEST);
   }

Can't we avoid to duplicate the functionality of both ControllerResolver and Httpkernel here? Why not just use HttpKernel->forward and change the request headers first?

Hm, is testbot stuck on this patch? Seems like it's been testing for the past day with no result.

+    $form['description'] = array(
+      '#markup' => '<p>' . t("Look out kid, it's somethin' you did, god knows when but you're doin' it again.") . '</p>',
+    );

Even though this is A) a test that few will ever see and B) a reference to a Bob Dylan song, I think it'd be safer of us to avoid any controversy by replacing this with a more generic message (bunnies and unicorns apparently widely accepted).

Wait is over :).

Uploading #8 do not test patch with suggestion in #11.
All above fails passes locally.

This passes locally.

This passes locally.

This is because the test assumes too much. It shouldn't assume either clean URLs or being installed at the root level of the host domain. Since testbot's root directory is /checkout/, not /, the test doesn't pass with action="/ajax-test/dialog" being hard-coded.

Same patch, but uses xpath instead of assertRaw against the HTML strings. This approach should make the tests less sensitive to breakage in the future, since they only check for a form with a certain ID being present, rather than exact HTML strings.

#20: dialog_routes-1998698-20.patch queued for re-testing.

Thanks @larowlan. I think I'm probably qualified to mark RTBC, having only fixed tests.

          // Generate a target based on the controller.
          $target = '#drupal-dialog-' . drupal_html_id(drupal_clean_css_identifier(drupal_strtolower($_content)));

DialogController::dialog() is still using the $_content variable which now does not exist...

+++ b/core/lib/Drupal/Core/Controller/DialogController.phpundefined
@@ -82,20 +83,18 @@ public function modal(Request $request, $_content) {
-      //  http://drupal.org/node/1871596 is in.
+      //   http://drupal.org/node/1871596 is in.

Unnecessary change

I don't know how to replace

        // Generate a target based on the controller.
          $target = '#drupal-dialog-' . drupal_html_id(drupal_clean_css_identifier(drupal_strtolower($_content)));

.
I hope interdiff is fine.

#27: dialog_routes-1998698-27.patch queued for re-testing.

Thanks @larowlan I tried rerolling this today but couldn't figure out for the life of me how to acquire the right variable there.

One minor nitpick is we're not using drupal_html_id() properly here:

+          $target = '#drupal-dialog-' . drupal_html_id(drupal_clean_css_identifier(drupal_strtolower($route_name)));

drupal_html_id() should be around the whole ID:

+          $target = '#' . drupal_html_id('drupal-dialog-' . drupal_clean_css_identifier(drupal_strtolower($route_name)));

Otherwise you may end up with $route_name matching some other ID already on the page just by mere coincidence, which results in "--2" being appended to the ID.

Let's do this instead $target = '#' . drupal_html_id("drupal-dialog-$route_name");

Thanks @larowlan I tried rerolling this today but couldn't figure out for the life of me how to acquire the right variable there.

So I was not alone :D

The feedback from alex got adressed so back to RTBC.

Committed f7f7f37 and pushed to 8.x. Thanks!