Use a route enhancer to handle _form, like everything else [#2613034]

Fork of #2595695: 4xx handling using subrequests: no longer able to vary by URL

The conversion of _form being in a request listener means it doesn't happen as part of routing, per se. That causes an issue for subrequests and cache contexts, as seen in #2595695: 4xx handling using subrequests: no longer able to vary by URL. The various entity magic keys are handled with a RouteEnhancer, which is part of routing.

So let's make _form do that, too. There should be no API change here, other than implicitly fixing a bug in #2595695: 4xx handling using subrequests: no longer able to vary by URL in a nicer way. (It's probably also immeasurably faster.)

Comment	File	Size	Author
#5	interdiff.txt	1.15 KB	Wim Leers
#5	2595695-move-form-5.patch	3.61 KB	Wim Leers
#5	8.0.x: PHP 5.5 & MySQL 5.5 14,349 pass
#4	2595695-move-form-4.patch	3.6 KB	Wim Leers
#4	8.0.x: PHP 5.5 & MySQL 5.5 14,347 pass
#3	2595695-move-form-3.patch	2.57 KB	Wim Leers
#3	8.0.x: PHP 5.5 & MySQL 5.5 10,957 pass, 1,941 fail
	2595695-move-form.patch	5.98 KB	Crell
	8.0.x: PHP 5.5 & MySQL 5.5 14,338 pass

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

11 November 2015 at 01:05

Crell created an issue. See original summary.

Comment #2

dawehner

German

CreditAttribution: dawehner as a volunteer commented 11 November 2015 at 10:22

Closing all the

+++ b/core/lib/Drupal/Core/Routing/Enhancer/FormRouteEnhancer.php
@@ -0,0 +1,33 @@
+/**
+ * Contains Drupal\Core\Routing\Enhancer\FormRouteEnhancer.
+ */

nitpick: @file, but note: #2304909: Relax requirement for @file when using OO Class or Interface per file is RTBC FUCK YEAH!!!!!!!!!!!!!!!!!!!!!!!!!

+++ b/core/lib/Drupal/Core/Routing/Enhancer/FormRouteEnhancer.php
@@ -0,0 +1,33 @@
+   * @inheritDoc
...
+   * @inheritDoc

Don't we use {@inheritdoc}
Did you typed that out, or is this coming from somewhere automatically?

+++ b/core/lib/Drupal/Core/Routing/Enhancer/FormRouteEnhancer.php
@@ -0,0 +1,33 @@
diff --git a/sites/default/default.settings.php b/sites/default/default.settings.php

diff --git a/sites/default/default.settings.php b/sites/default/default.settings.php
index 1a6d148..b1f95e2 100644

index 1a6d148..b1f95e2 100644
--- a/sites/default/default.settings.php

--- a/sites/default/default.settings.php
+++ b/sites/default/default.settings.php

+++ b/sites/default/default.settings.php
+++ b/sites/default/default.settings.php
@@ -223,24 +223,22 @@

Meh, out of scope

Comment #3

Wim Leers

Ghent 🇧🇪🇪🇺

CreditAttribution: Wim Leers as a volunteer commented 11 November 2015 at 12:37

Assigned:

Unassigned

» Wim Leers

File	Size
2595695-move-form-3.patch	2.57 KB
8.0.x: PHP 5.5 & MySQL 5.5 10,957 pass, 1,941 fail

1 file was hidden/shown/deleted

File	Size
2595695-move-form.patch	5.98 KB
8.0.x: PHP 5.5 & MySQL 5.5 14,338 pass

First rerolling without the default.settings.php changes.

Comment #4

Wim Leers

Ghent 🇧🇪🇪🇺

CreditAttribution: Wim Leers as a volunteer commented 11 November 2015 at 12:42

File	Size
2595695-move-form-4.patch	3.6 KB
8.0.x: PHP 5.5 & MySQL 5.5 14,347 pass

1 file was hidden/shown/deleted

File	Size
2595695-move-form-3.patch	2.57 KB
8.0.x: PHP 5.5 & MySQL 5.5 10,957 pass, 1,941 fail

Ugh, forgot to include the new file.

Comment #5

Wim Leers

Ghent 🇧🇪🇪🇺

CreditAttribution: Wim Leers as a volunteer commented 11 November 2015 at 12:43

Assigned:

Wim Leers

» Unassigned

File	Size
2595695-move-form-5.patch	3.61 KB
8.0.x: PHP 5.5 & MySQL 5.5 14,349 pass
interdiff.txt	1.15 KB

1 file was hidden/shown/deleted

File	Size
2595695-move-form-4.patch	3.6 KB
8.0.x: PHP 5.5 & MySQL 5.5 14,347 pass

#2 all fixed, and fixed additional nitpicks too.

Also: #2304909: Relax requirement for @file when using OO Class or Interface per file <3 <3 <3 YES!!

Comment #6

11 November 2015 at 12:50

The last submitted patch, 3: 2595695-move-form-3.patch, failed testing.

Comment #7

Crell CreditAttribution: Crell at Palantir.net commented 11 November 2015 at 16:18

Eh, sorry for the jitter. It's been a while since I rolled a Drupal patch rather than a PR on something. :-P But yes, let's do!

Comment #8

dawehner

German

CreditAttribution: dawehner as a volunteer commented 11 November 2015 at 16:19

Status:

Needs review

» Reviewed & tested by the community

+++ /dev/null
@@ -1,48 +0,0 @@
- *   https://www.drupal.org/node/2092647 has landed.

Yeah, one less todo :)

Comment #9

tim.plunkett

he/him

English

Philadelphia

CreditAttribution: tim.plunkett as a volunteer commented 11 November 2015 at 16:30

Issue tags:

+DX (Developer Experience)

Comment #10

catch

he/him

English

CreditAttribution: catch commented 12 November 2015 at 17:56

Issue tags:

+rc target

This blocks #2595695: 4xx handling using subrequests: no longer able to vary by URL which is an RC target.

It removes a service which theoretically someone could rely on, but as of this morning, event subscribers are treated as @internal per https://www.drupal.org/node/2562903/.

It's a real shame we didn't do #2350491: Discuss/implement our usage of private services then it would not even be a question of whether someone's using it or not except for subclassing - I've just re-opened that issue since I can't find the issue it was marked duplicate of.

Comment #11

catch

he/him

English

CreditAttribution: catch commented 12 November 2015 at 20:34

Status:

Reviewed & tested by the community

» Fixed

Committed/pushed to 8.0.x, thanks!

Comment #12

19 November 2015 at 17:53

catch committed 7e3042e on 8.1.x

Issue #2613034 by Wim Leers, Crell: Use a route enhancer to handle _form...

Comment #13

Crell CreditAttribution: Crell at Palantir.net commented 20 November 2015 at 05:35

#11 says this was committed to 8.0.x, meaning it should be in today's 8.0.0 tag. The automated message in #12, though, says it's for 8.1.x. Where'd it go? :-)