Download & Extend

drupal_encode_path (former drupal_urlencode) and commas

Project:Drupal core
Version:8.x-dev
Component:base system
Category:feature request
Priority:normal
Assigned:Unassigned
Status:needs review

Issue Summary

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.

AttachmentSizeStatusTest resultOperations
drupal_urlencode_comma.patch697 bytesIdleFAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal_urlencode_comma.patch.View details | Re-test

Comments

#1

Status:active» needs review

#2

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

#3

Version:5.x-dev» 6.x-dev
Status:needs review» needs work

No longer applies.

Needs to be patched in 6.x first.

#4

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 "+".

AttachmentSizeStatusTest resultOperations
drupal_urlencode.patch583 bytesIdleFAILED: [[SimpleTest]]: [MySQL] Invalid patch format in drupal_urlencode_1.patch.View details | Re-test

#5

Version:6.x-dev» 8.x-dev
Category:bug report» feature request
Priority:minor» normal
Status:needs work» patch (to be ported)

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

#6

Ok, i didnt know that.

Thanks.

#7

Status:patch (to be ported)» needs review

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.

AttachmentSizeStatusTest resultOperations
113035-7_drupal_urlencode_commas.patch877 bytesIdleFAILED: [[SimpleTest]]: [MySQL] 29,458 pass(es), 3 fail(s), and 0 exception(es).View details | Re-test

#8

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().

<?php

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);
?>

#9

Status:needs review» needs work

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

#10

Correcting test.

AttachmentSizeStatusTest resultOperations
113035-10_drupal_urlencode_commas.patch2.12 KBIdleFAILED: [[SimpleTest]]: [MySQL] 29,460 pass(es), 2 fail(s), and 0 exception(es).View details | Re-test

#11

Status:needs work» needs review

#12

Status:needs review» needs work

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

#13

Status:needs work» needs review

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.

<?php
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;
}
?>
AttachmentSizeStatusTest resultOperations
113035-13_drupal_encode_path_optional_unencode.patch2.83 KBIdlePASSED: [[SimpleTest]]: [MySQL] 29,464 pass(es).View details | Re-test

#14

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.

#15

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

nobody click here