741171 - When clicking on a user name in show_bug.cgi, user activity doesn't work when email contains plus (+) - email address needs to be encoded

Status: RESOLVED FIXED

(bugzilla.mozilla.org :: Extensions, defect)

Description

[Magnus Melin [:mkmelin]]

The user activity link doesn't find anything when the user email contains +. 

E.g. https://bugzilla.mozilla.org/page.cgi?id=user_activity.html&action=run&from=2012-03-18&to=2012-04-01&who=[e-mail]  -- nothing.

The email address needs to be encoded (+ is interpreted as a space).

Comment 1

[David Lawrence [:dkl]]

Attachment: Patch to allow + in email for user activity report (v1)

Comment 2

[Reed Loden [:reed]]

Comment on attachment 611576 [details] [diff] [review]
Patch to allow + in email for user activity report (v1)

Review of attachment 611576 [details] [diff] [review]:
-----------------------------------------------------------------

This seems like more of a job for escape(). However, as far as I can tell, we don't do this same type of replacing in other fields, so what makes this particular one so special?

Comment 3

[David Lawrence [:dkl]]

Attachment: Patch to allow + in email for user activity report (v2)

Changed to use encodeURIComponent instead of using replace(). escape and encodeURI will preserve '+' so that would not work for this particular issue. I am only altering the who portion of the form as email addresses should be treated differently with regard to + characters and should matter to the other form fields. CGI.pm explicitly converts '+' to spaces when decoding the form values so we have to encode before submission.

dkl

Comment 4

[:glob ✱]

Comment on attachment 611589 [details] [diff] [review]
Patch to allow + in email for user activity report (v2)

i don't think this is a bug; however if it was this isn't the right fix.  it needs to not rely on javascript, and be fixed in the userselect field upstream.

Comment 5

[:glob ✱]

i suspect magnus is reporting about a problem with the url in the popup menu you get when clicking on a user, not in the report itself.

Comment 6

[:glob ✱]

oops, didn't mean to close this

Comment 7

[David Lawrence [:dkl]]

Attachment: Patch to allow + in email for user activity report (v3)

Comment 8

[Reed Loden [:reed]]

Comment on attachment 612458 [details] [diff] [review]
Patch to allow + in email for user activity report (v3)

Review of attachment 612458 [details] [diff] [review]:
-----------------------------------------------------------------

::: extensions/BMO/web/js/edituser_menu.js
@@ +22,1 @@
>    admin_usermenu.getItem(2).cfg.setProperty('url', 'mailto:' + escape(email));

Why does this one stay escape() when you change the other one above?

Comment 9

[:glob ✱]

Comment on attachment 612458 [details] [diff] [review]
Patch to allow + in email for user activity report (v3)

Review of attachment 612458 [details] [diff] [review]:
-----------------------------------------------------------------

as reed pointed out, you need to change both instances of escape() to encodeURIComponent(), please fix on commit.

r=glob

Comment 10

[David Lawrence [:dkl]]

Thanks

Committing to: bzr+ssh://[e-mail]/bmo/4.0
modified extensions/BMO/web/js/edituser_menu.js
Committed revision 8120.
                                                          
Committing to: bzr+ssh://[e-mail]/bmo/4.2
modified extensions/BMO/web/js/edituser_menu.js
Committed revision 8106.

dkl