Support from Acquia helps fund testing for Drupal Acquia logo

Comments

aaronott’s picture

Assigned: Unassigned » aaronott
Status: Active » Needs review
FileSize
695 bytes
aaronott’s picture

Assigned: aaronott » Unassigned

Status: Needs review » Needs work

The last submitted patch, 1999344_1-replace-raw-variables-edit.patch, failed testing.

arknoll’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 1999344_1-replace-raw-variables-edit.patch, failed testing.

aaronott’s picture

This time with PSR-0 Format

aaronott’s picture

Status: Needs work » Needs review
kim.pepper’s picture

Status: Needs review » Needs work
--- a/core/modules/edit/lib/Drupal/edit/EditController.php
+++ b/core/modules/edit/lib/Drupal/edit/EditController.phpundefined

@@ -126,7 +126,7 @@ public function fieldForm(EntityInterface $entity, $field_name, $langcode, $view
+    if (\Drupal::request()->request->get('nocssjs') === 'true') {

As this is a method being called in a controller route, we can just pass in Request as a magic parameter, instead of using \Drupal::request()

aaronott’s picture

@kim.pepper

I'm not sure how to do this or what you mean by passing in Request as a magic parameter. Do you have an example or link that I can read up on?

kim.pepper’s picture

Have a look at \Drupal\aggregator\Routing\AggregatorController::feedRefresh() http://drupalcode.org/project/drupal.git/blob/HEAD:/core/modules/aggrega...

The Request parameter can be passed into the method (in any order) and you don't need to specify it in your route configuration.

aaronott’s picture

Okay so I think I've got this.

Now allowing for the Request parameter to be passed in.

kim.pepper’s picture

Looking good!

One minor nitpick:

+++ b/core/modules/edit/lib/Drupal/edit/EditController.phpundefined
@@ -93,10 +93,12 @@ public function metadata(Request $request) {
+   * @param \Symfony\Component\HttpFoundation\Request $request
+   *   The current request object containing the search string.

Should have a new line after last param comment

kim.pepper’s picture

double post

aaronott’s picture

I thought so but missed it. here it is again. Thanks!

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Looks great!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed e0db7eb and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.