Hi,
The function implementing hook_menu in commerce_cart.module is having this item in the $items array:
$items['cart/my']

This item will redirect to 'cart' path.

It is using a drupal_goto function, and no alias option parameter is being used (I don´t know if that was the intention). This way, if I type in the browser:
http://example.org/cart/my/%
% = strings, integers,...

I'will get this error:

Recoverable fatal error: Argument 2 passed to drupal_goto() must be an array, string given in drupal_goto() (line 663 de /home/example/includes/common.inc)

I really like how commerce is doing, but I am not really get used to it (just begining). I will add this patch, so you guys could point me if this is the right direction. I am not also familiar with hook_menu and how drupal_goto really works, so reviews, comments... would be great.

CommentFileSizeAuthor
#3 1420698-3.patch910 bytesamateescu
#1 commerce-commerce_cart_menu_drupal_goto_arguments-1420698-1.patch574 bytestuwebo
commerce-commerce_cart_menu_drupal_goto_arguments-0.patch574 bytestuwebo

Comments

tuwebo’s picture

tuwebo commented
Status: Active » Needs review
StatusFileSize
new commerce-commerce_cart_menu_drupal_goto_arguments-1420698-1.patch574 bytes

Changing patch name and setting proper status.

tuwebo’s picture

tuwebo commented
Title: Commerce cart menu, "cart/my" missing option alias array in drupal_goto page callback » Adding arguments to "cart/my" url breaks the site

Changing to a better title

amateescu’s picture

amateescu commented
Issue tags: -+patch
StatusFileSize
new 1420698-3.patch910 bytes

I don't really see why someone would add arguments to that url, but 'fixing' this behavior won't hurt either :)

Anyway, the patch from #1 doesn't take into account that someone could give even more parameters to drupal_goto() if they enter more arguments (e.g. cart/my/arg1/arg2), so I propose something like the patch attached.

tuwebo’s picture

tuwebo commented

Hi,
amateescu thanks for the fast response.
Both patches #1 and #3 worked for me, with any kind and number of arguments I have tested.

It looks good to me and working fine!

pcambra’s picture

pcambra
he/him
Primary language Spanish
commented

Is it cart/my actually used or needed? It seems to me that we could even drop the whole menu item, not sure what is it purpose.

rszrama’s picture

rszrama commented
Status: Needs review » Fixed

Great solution, amateescu.

@pcambra It's a very common feature request to have a menu item with a dynamic title, which is what cart/my is. It was added in a while back and is still useful. : )

Status: Fixed » Closed (fixed)

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