This is just a pretty-it-up patch for drupal_urlencode (which I'm hoping shouldn't change functionality).
Currently commas in urls get escaped, which makes multiple taxonomy selection urls look ugly. This patch makes sure the comma doesn't get escaped. (example: taxonomy/term/5,16,37)

It should be valid against both 5.x-dev and head.

CommentFileSizeAuthor
#13 113035-13_drupal_encode_path_optional_unencode.patch2.83 KBwojtha
#10 113035-10_drupal_urlencode_commas.patch2.12 KBwojtha
#7 113035-7_drupal_urlencode_commas.patch877 byteswojtha
#4 drupal_urlencode.patch583 bytesludo.r
drupal_urlencode_comma.patch697 bytesmoonray

Comments

moonray’s picture

moonray commented
Status: Active » Needs review
Steven’s picture

Steven commented

Which characters are allowed to be unescaped in a URL depends on the location in the URL. If we add such pretty printing, it needs to default to off, or otherwise drupal_urlencode() becomes useless in some situations. I think the best solution is a list of characters not to encode, passed as an array.

http://gbiv.com/protocols/uri/rfc/rfc3986.html#reserved

drumm’s picture

drumm
he/him
Location NY, US
commented
Version: 5.x-dev » 6.x-dev
Status: Needs review » Needs work

No longer applies.

Needs to be patched in 6.x first.

ludo.r’s picture

ludo.r
Location Brussels
commented
StatusFileSize
new drupal_urlencode.patch583 bytes

This is an old issue, but it is still unpatched.

Please see this thread for more information : http://drupal.org/node/781192

I've tested a little patch (almost the same as #1), it corrects commas "," and pluses "+".

damien tournoud’s picture

damien tournoud commented
Version: 6.x-dev » 8.x-dev
Category: bug » feature
Priority: Minor » Normal
Status: Needs work » Patch (to be ported)

This is a feature request. Only 8.x is currently opened to those.

ludo.r’s picture

ludo.r
Location Brussels
commented

Ok, i didnt know that.

Thanks.

wojtha’s picture

wojtha commented
Status: Patch (to be ported) » Needs review
StatusFileSize
new 113035-7_drupal_urlencode_commas.patch877 bytes

Running today in the same issue in D6.

URLs like taxonomy/term/123%456C2%2C789 instead of taxonomy/term/123,456,789 are really ugly.

wojtha’s picture

wojtha commented
Title: drupal_urlencode and commas » drupal_encode_path (former drupal_urlencode) and commas

Fixing the title - drupal_urlencode was replaced by drupal_encode_path in D7+.

Is this really so big feature so it can't be backported?

I was thinking about fixing this in D7 using contrib module. Unfortunetally hook_drupal_goto_alter is called before the url() and drupal_encode_path().

function drupal_goto($path = '', array $options = array(), $http_response_code = 302) {  
  // ... code ...
  drupal_alter('drupal_goto', $path, $options, $http_response_code);

  // The 'Location' HTTP header must be absolute.
  $options['absolute'] = TRUE;

  $url = url($path, $options);

Status: Needs review » Needs work

The last submitted patch, 113035-7_drupal_urlencode_commas.patch, failed testing.

wojtha’s picture

wojtha commented
StatusFileSize
new 113035-10_drupal_urlencode_commas.patch2.12 KB

Correcting test.

wojtha’s picture

wojtha commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 113035-10_drupal_urlencode_commas.patch, failed testing.

wojtha’s picture

wojtha commented
Status: Needs work » Needs review
StatusFileSize
new 113035-13_drupal_encode_path_optional_unencode.patch2.83 KB

Hmm I see, allowing filenames to have commas and pluses is not nice...

I tried alternative approach, with the following patch additional "ignored" characters are passed as optional parameter of the drupal_encode_path(). +1 to this approach is also the reason that other ignored characters will be used only occassionally.

function drupal_encode_path($path, $no_encode_chars = array()) {
  $path = str_replace('%2F', '/', rawurlencode($path));
  if (!empty($no_encode_chars)) {
    $path = str_replace(array_map('rawurlencode', $no_encode_chars), $no_encode_chars, $path);
  }
  return $path;
}
anou’s picture

anou
Primary language French
Location Collonges au Mont d'Or
commented

Hello,
I like your approach, but I'd like to know where and/or when do you define $no_encode_chars ?
In a page_preprocess function ? In a module ?

My task was trying to let "?" and "=" not encoded when using hook_menu_breadcrumb_alter() in a custom module.

No success for the moment without modifying drupal_encode_path() directly in common.inc.

wojtha’s picture

wojtha commented

@anou with my patch you can set $options['no_encode_chars'] array at url() or at l() function.

thedavidmeister’s picture

thedavidmeister commented

#13: 113035-13_drupal_encode_path_optional_unencode.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 113035-13_drupal_encode_path_optional_unencode.patch, failed testing.

ñull’s picture

ñull commented
Issue summary: View changes

The issue is here presented as "ugly" but nobody addressed the usability aspect. The resulting encoded URL is made public in the browser address field (the browser does not decode them). The ugly but valid URL can then be copied and pasted for instance in a Drupal menu path ( direct link to a often used tag query).

Now guess what happens when you do that. The encoded URL with pluses or commas is not recognised as valid URL! For an average user difficult to understand why it would not be valid and then forget about explaining how to decode the URL to make it work again. Something is really wrong here from a usability perspective. If you apparently oblige coded URLs with drupal_goto then they should be accepted everywhere!

ñull’s picture

ñull commented
Title: drupal_encode_path (former drupal_urlencode) and commas » drupal_encode_path (former drupal_urlencode) handling commas and pluses
gavsiu’s picture

gavsiu commented

[deleted]

(no button for actual delete)

mgifford’s picture

mgifford
he/him
Primary language English
commented
Issue tags: +drupal_encode_path
Related issues: +#2447301: UrlHelper::encodePath() doesn't support Windows paths

Does drupal_encode_path() exist on D8 any more?

EDIT: Replaced with function encodePath($path)

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.

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

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

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

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

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

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

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

smustgrave commented
Status: Needs work » Postponed (maintainer needs more info)
Issue tags: +stale-issue-cleanup

Thank you for sharing your idea for improving Drupal.

We are working to decide if this proposal meets the Criteria for evaluating proposed changes. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or there is no community support. Your thoughts on this will allow a decision to be made.

Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

Thanks!

smustgrave’s picture

smustgrave commented
Status: Postponed (maintainer needs more info) » Closed (outdated)

Since drupal_encode_path has since been deprecated and removed. And this was request vs task going to close out. But if still needed please re-open.

Thanks all