PHP 5.4 is required now for Drupal8 so we can get rid of this method.

CommentFileSizeAuthor
#54 2322059-54.patch704 bytesgriffynh
#48 2322059-47.interdiff.txt8.03 KBclaudiu.cristea
#48 2322059-47.patch30.11 KBclaudiu.cristea
#48 2248257-40.interdiff.txt8.03 KBclaudiu.cristea
#45 2322059-45.patch23.04 KBclaudiu.cristea
#35 urlhelper-buildquery-2322059-34.patch1.76 KBsarhugo
#31 urlhelper-buildquery-2322059-31.patch1.76 KBsarhugo
#28 2322059-28.patch16.07 KBravi.khetri
#23 2322059-23.patch16.6 KBravi.khetri
#21 2322059-21.patch16.05 KBadci_contributor
#17 2322059-13.patch16.36 KBAshok Negi
#13 2322059-13.patch15.98 KBrpayanm
#11 2322059-9.patch14.6 KBtibbsa
#8 2322059-8.patch14.6 KBrpayanm
#4 2322059-4.patch14.6 KBrpayanm
#1 urlhelper_buildquery-2322059-1.patch12.97 KBundertext

Comments

undertext’s picture

undertext commented
Status: Active » Needs review
StatusFileSize
new urlhelper_buildquery-2322059-1.patch12.97 KB

Status: Needs review » Needs work

The last submitted patch, 1: urlhelper_buildquery-2322059-1.patch, failed testing.

mgifford’s picture

mgifford
he/him
Primary language English
commented
Issue tags: +Needs reroll
rpayanm’s picture

rpayanm
he/him
Primary language Spanish
commented
Status: Needs work » Needs review
StatusFileSize
new 2322059-4.patch14.6 KB

rerolling...

Status: Needs review » Needs work

The last submitted patch, 4: 2322059-4.patch, failed testing.

Status: Needs work » Needs review

rpayanm queued 4: 2322059-4.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 4: 2322059-4.patch, failed testing.

rpayanm’s picture

rpayanm
he/him
Primary language Spanish
commented
Status: Needs work » Needs review
StatusFileSize
new 2322059-8.patch14.6 KB

trying again...

rpayanm’s picture

rpayanm
he/him
Primary language Spanish
commented

ummm 8.1.x-dev or would be 8.0.x-dev?

Status: Needs review » Needs work

The last submitted patch, 8: 2322059-8.patch, failed testing.

tibbsa’s picture

tibbsa commented
Version: 8.1.x-dev » 8.0.x-dev
Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new 2322059-9.patch14.6 KB

This applies to the latest 8.0.x code -- trying again to get it tested.

Status: Needs review » Needs work

The last submitted patch, 11: 2322059-9.patch, failed testing.

rpayanm’s picture

rpayanm
he/him
Primary language Spanish
commented
Status: Needs work » Needs review
StatusFileSize
new 2322059-13.patch15.98 KB

Status: Needs review » Needs work

The last submitted patch, 13: 2322059-13.patch, failed testing.

Status: Needs work » Needs review

rpayanm queued 13: 2322059-13.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 13: 2322059-13.patch, failed testing.

Ashok Negi’s picture

Ashok Negi commented
StatusFileSize
new 2322059-13.patch16.36 KB

Attached patch

Ashok Negi’s picture

Ashok Negi commented
Status: Needs work » Needs review
Issue tags: +#dcdelhi

Status: Needs review » Needs work

The last submitted patch, 17: 2322059-13.patch, failed testing.

rpayanm’s picture

rpayanm
he/him
Primary language Spanish
commented
Issue tags: +Needs reroll
adci_contributor’s picture

adci_contributor commented
Status: Needs work » Needs review
StatusFileSize
new 2322059-21.patch16.05 KB

Trying to reroll

Status: Needs review » Needs work

The last submitted patch, 21: 2322059-21.patch, failed testing.

ravi.khetri’s picture

ravi.khetri commented
StatusFileSize
new 2322059-23.patch16.6 KB

Attached patch

ravi.khetri’s picture

ravi.khetri commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 23: 2322059-23.patch, failed testing.

Status: Needs work » Needs review

ravi.khetri queued 23: 2322059-23.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 23: 2322059-23.patch, failed testing.

ravi.khetri’s picture

ravi.khetri commented
Status: Needs work » Needs review
StatusFileSize
new 2322059-28.patch16.07 KB

Status: Needs review » Needs work

The last submitted patch, 28: 2322059-28.patch, failed testing.

jlbellido’s picture

jlbellido
Primary language Spanish
commented
Issue tags: +SprintWeekend2015

Working on this issue for #sprintweekend2015

sarhugo’s picture

sarhugo commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new urlhelper-buildquery-2322059-31.patch1.76 KB

The patch fails because of a wrong placed parenthesis:

Line 650 of core/lib/Drupal/Core/Menu/MenuTreeStorage.php

instead of

$param_key = $route_parameters ? http_build_query(UrlHelper::filterQueryParameters($route_parameters, NULL, NULL, PHP_QUERY_RFC3986)) : '';

should be

$param_key = $route_parameters ? http_build_query(UrlHelper::filterQueryParameters($route_parameters), NULL, NULL, PHP_QUERY_RFC3986) : '';

But instead of replacing every call of UrlHelper::buildQuery to http_build_query, why not doing UrlHelper::buildQuery as a wrapper that calls http_build_query with the correct encoding.
Attached is a patch showing what I mean.

Status: Needs review » Needs work

The last submitted patch, 31: urlhelper-buildquery-2322059-31.patch, failed testing.

Status: Needs work » Needs review

sarhugo queued 31: urlhelper-buildquery-2322059-31.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 31: urlhelper-buildquery-2322059-31.patch, failed testing.

sarhugo’s picture

sarhugo commented
Status: Needs work » Needs review
StatusFileSize
new urlhelper-buildquery-2322059-34.patch1.76 KB

Seems that http_build_query takes null as an empty string in arg_separator parameter.
This patches fixes it.

Status: Needs review » Needs work

The last submitted patch, 35: urlhelper-buildquery-2322059-34.patch, failed testing.

sarhugo’s picture

sarhugo commented

The failing tests show that UrlHelper::buildQuery and http_build_query don't give the same results.

  • For null values http_build_query omits the key while buildQuery adds it without value
  • For nested arrays http_build_query encodes the square brackets while buildQuery doesn't
  • For values with slashes (not keys) http_build_query encodes the slashes while buildQuery doesn't

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Version: 8.6.x-dev » 8.8.x-dev
claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Component: other » base system
Status: Needs work » Needs review
Issue tags: -#dcdelhi, -SprintWeekend2015
StatusFileSize
new 2322059-45.patch23.04 KB

Reroll. Let's see again which tests are failing.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Title: Remove usage of UrlHelper::buildQuery method » Remove usage and deprecate UrlHelper::buildQuery method
Issue tags: +@deprecated

Status: Needs review » Needs work

The last submitted patch, 45: 2322059-45.patch, failed testing. View results

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Status: Needs work » Needs review
StatusFileSize
new 2248257-40.interdiff.txt8.03 KB
new 2322059-47.patch30.11 KB
new 2322059-47.interdiff.txt8.03 KB

Fixing some BigPipe tests.

Status: Needs review » Needs work

The last submitted patch, 48: 2322059-47.patch, failed testing. View results

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

I talked to alexpott about this a while ago and he said we might want to close this as a won't fix, the functions have different output and changing that could be problematic.

we should decide on that first before we continue to work on it.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Related issues: +#2248257: Optimize UrlHelper::buildQuery() to use http_build_query()

@Berdir, OK. Note that #2248257: Optimize UrlHelper::buildQuery() to use http_build_query() is related and, probably, we want to close that too.

PS: However, I'm not convinced that we should keep it.

neclimdul’s picture

neclimdul
He/Him
commented

urlHelper::buildQuery subtly does more that is obvious by reading the code as was documented in the other issue. I'm going to suggest the method be un-deprecated and the be closed as works by design.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

griffynh’s picture

griffynh commented
Title: Remove usage and deprecate UrlHelper::buildQuery method » Keep the UrlHelper::buildQuery method and removed @todo
Status: Needs work » Needs review
Issue tags: +DrupalSouth 2019
StatusFileSize
new 2322059-54.patch704 bytes

As per #52, 50 - We are going to keep the function, so I've removed the @todo

Please find attached the new patch.

Thanks to @jibran for mentoring me on this patch.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Title: Keep the UrlHelper::buildQuery method and removed @todo » Keep the UrlHelper::buildQuery method and remove @todo
Status: Needs review » Reviewed & tested by the community

This looks perfect. Thanks, @griffynh

  • xjm committed 7f44b45 on 9.0.x 
    Issue #2322059 by rpayanm, claudiu.cristea, ravi.khetri, sarhugo,...

  • xjm committed ac5e456 on 8.9.x 
    Issue #2322059 by rpayanm, claudiu.cristea, ravi.khetri, sarhugo,...

  • xjm committed 035a770 on 8.8.x 
    Issue #2322059 by rpayanm, claudiu.cristea, ravi.khetri, sarhugo,...
xjm’s picture

xjm
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed to 9.0.x, 8.9.x, and 8.8.x live at DrupalSouth!!

Status: Fixed » Closed (fixed)

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