Part of #1998638: Replace almost all remaining superglobals ($_GET, $_POST, etc.) with Symfony Request object

Files that need converting are:

  • core/modules/comment/comment.admin.inc
  • core/modules/comment/comment.module
  • core/modules/comment/comment.pages.inc
Files: 
CommentFileSizeAuthor
#5 1999340-comment-request-5.patch2.17 KBkim.pepper
PASSED: [[SimpleTest]]: [MySQL] 57,750 pass(es).
[ View ]
#5 interdiff.txt1.05 KBkim.pepper
#1 1999340_1-replace-raw-variables-comment.patch1.88 KBchertzog
PASSED: [[SimpleTest]]: [MySQL] 55,912 pass(es).
[ View ]

Comments

Assigned:Unassigned» chertzog
Status:Active» Needs review
StatusFileSize
new1.88 KB
PASSED: [[SimpleTest]]: [MySQL] 55,912 pass(es).
[ View ]

Status:Needs review» Needs work

The last submitted patch, 1999340_1-replace-raw-variables-comment.patch, failed testing.

Status:Needs work» Needs review

Status:Needs review» Needs work

+++ b/core/modules/comment/comment.module
@@ -434,7 +434,7 @@ function comment_permalink(Comment $comment) {
-    $_GET['page'] = $page;
+    Drupal::request()->query->set('page', $page);
     return drupal_container()->get('http_kernel')->handle($subrequest, HttpKernelInterface::SUB_REQUEST);

Slight scope creep, but as long as we're in here let's change drupal_container()->get() to Drupal::service('http_kernel').

Status:Needs work» Needs review
StatusFileSize
new1.05 KB
new2.17 KB
PASSED: [[SimpleTest]]: [MySQL] 57,750 pass(es).
[ View ]

Fix in #4 plus replaced uses of deprecated drupal_container().

Status:Needs review» Needs work

The last submitted patch, 1999340-comment-request-5.patch, failed testing.

"Failed to write configuration file" Re-testing.

Status:Needs work» Needs review

#5: 1999340-comment-request-5.patch queued for re-testing.

Status:Needs review» Reviewed & tested by the community

Status:Reviewed & tested by the community» Fixed

Committed to 8.x. Thanks!

+++ b/core/modules/comment/comment.module
@@ -430,12 +430,12 @@ function comment_permalink(Comment $comment) {
     // @todo: Convert the pager to use the request object.
-    $_GET['page'] = $page;
-    return drupal_container()->get('http_kernel')->handle($subrequest, HttpKernelInterface::SUB_REQUEST);
+    $request->query->set('page', $page);
+    return Drupal::service('http_kernel')->handle($subrequest, HttpKernelInterface::SUB_REQUEST);

Should we remove that @todo now or is there something else needed for that?

I think that would require rewriting the pager system, which has needed one for a few years now but still hasn't gotten it. :-(

Does that mean the patch that got committed broke the pager when used on a page with comments? Sorry, I'd check myself, but trying to focus on other issues, so leaving this comment in case someone else wants to investigate.

Status:Fixed» Closed (fixed)

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