634243 - using "confirm match" leads to match failed error due to an added comma

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[bigstijn]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.13) Gecko/20101203 Firefox/3.6.13
Build Identifier: Bugzilla 4.0 / tip

using "confirm match" leads to match failed error due to an added comma.

" Assignee: [e-mail], did not match anything "

tested on landfill 4.0 and tip .

Reproducible: Always

Steps to Reproduce:
1. add a non-unique string in the assignee field
2. don't use the autocomplete
3. in the "confirm match" screen, choose an assignee
Actual Results:  
 Assignee: [e-mail], did not match anything

Expected Results:  
Assignee choosen in the drop-downbox is accepted

Comment 1

[bigstijn]

Attachment: confirm match screen

Comment 2

[bigstijn]

Attachment: match failed screen

Comment 3

[Frédéric Buclin]

Let's mark it as a blocker for now. I will investigate when I'm back home in an hour or so. mkanat, let me investigate before we release 4.0 as I suspect it's a regression.

Comment 4

[Max Kanat-Alexander]

Confirmed.

Comment 5

[Max Kanat-Alexander]

Attachment: v1

I haven't researched to figure out where this was introduced, but there seem to be two problems:

1) User::match is looking for multiple values for fields where there can only be one value.

2) confirm-user-match was sending back an empty "assigned_to" (etc.) as part of hidden-fields.html.tmpl, which meant that there were two assigned_to fields on the page.

I solved #2, because that seemed like a simple fix, and it seemed like a clear bug that confirm-user-match was sending back an extra, empty field value.

Comment 6

[Max Kanat-Alexander]

This was most likely regressed by bug 585802, although in a sense, it only exposed a bug in confirm-user-match.

Comment 7

[Max Kanat-Alexander]

The bug in confirm-user-match was hidden before because we joined fields by a space--putting an extra space at the end of the assigned_to or qa_contact fields didn't matter. But now we only accept commas as separators, and adding a comma does matter.

Comment 8

[Frédéric Buclin]

(In reply to comment #5)
> 1) User::match is looking for multiple values for fields where there can only
> be one value.

We force it to take all values into account even with type='single', because we don't want to choose the (wrong) value randomly. I spent enough time on this method in Bugzilla 2.20 and in bug 534057 to know that's the correct behavior. :)


> 2) confirm-user-match was sending back an empty "assigned_to" (etc.) as part of
> hidden-fields.html.tmpl, which meant that there were two assigned_to fields on
> the page.

This is indeed the reason of the problem, yes. Nice catch! :)

Comment 9

[Frédéric Buclin]

Comment on attachment 512478 [details] [diff] [review]
v1

Works fine. Thanks. r=LpSolit

Comment 10

[Max Kanat-Alexander]

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/                       
modified template/en/default/global/confirm-user-match.html.tmpl
Committed revision 7717.

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/4.0/                         
modified template/en/default/global/confirm-user-match.html.tmpl
Committed revision 7557.