-
Notifications
You must be signed in to change notification settings - Fork 241
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
customer order view workaround for "+" in email addresses #1199
Conversation
…esses that include a "+" character
+0,5 on the principle |
{ | ||
$Email = $http->getVariable( "email" ); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But, if i'm not wrong, here you are remove Email Param from the url, right? i mean, this is not valid as you won't have Email as Param anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I use a normal "get" parameter in orderlist.tpl, because "+" get through with them. I give this parameter a higher priority in the customerorderview.php. Does that seem wrong to you, @crevillo?
Of course fixing the bug (?) with $Params would be the better solution, but I don't know the consequences of doing that...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not very happy with that patch, because the real problem lies in $Params (and a missing urlencode()
). ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you have in mind changing it to?
as for Params, it kind of still need to be there for bc, like is the case now in 6f5f894
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I fail to understand what you are talking about. :)
Changing what to? And what "bc"?
I created an issue now, because I get no reaction on the pull request... |
Moved to eZ Publish Project (EZP): https://jira.ez.no/browse/EZP-24798 |
any news here? |
@andrerom I just realised that if you push changes to your own fork an open pull request gets updated automatically... Since I want to create a pull request for fixing PHP 7.2 deprecations warning/errors I had to remove my fork, since you cannot create a new one... |
hmm, should be possible, you just need to use different branches, or? |
ah, that sounds right. so I could have created another branch from |
Yes, thank you. I guess it's best always to create a branch for a new pull request! |
Pro tip: When you fork something, remove master from your fork so local master tracks "upstream" only, and you don't need to keep on updating your own master. As a result instead you always start new work from the "real" master from the upstream project ;) Doesn't fit in all cases, but in most it simplifies things and forces you to think in terms of one branch per feature as well ;) |
Do you mean my locally checked out master branch or the remote master branch (of my fork)? |
I added a workaround to the get the customer order view working with email addresses that include a "+" character.
First I thought only an "urlencode" was missing, but then I realized it wasn't possible to get a "+" across $Params.