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

RESOLVED FIXED in Bugzilla 4.0

Status

()

defect
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: bigstijn, Assigned: mkanat)

Tracking

({regression})

Dependency tree / graph
Bug Flags:
approval +
approval4.0 +
blocking4.0 +
testcase +

Details

Attachments

(3 attachments)

Reporter

Description

8 years ago
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: bigstijn@yahoo.co.uk, 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: bigstijn@yahoo.co.uk, did not match anything

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

Comment 1

8 years ago
Posted image confirm match screen
Reporter

Comment 2

8 years ago
Posted image match failed screen
Reporter

Updated

8 years ago
Flags: blocking4.0?

Comment 3

8 years ago
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.
Flags: blocking4.0? → blocking4.0+
Target Milestone: --- → Bugzilla 4.0
Version: unspecified → 4.0

Updated

8 years ago
Flags: testcase?
Assignee

Comment 4

8 years ago
Confirmed.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee

Comment 5

8 years ago
Posted patch v1Splinter Review
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.
Assignee: create-and-change → mkanat
Status: NEW → ASSIGNED
Attachment #512478 - Flags: review?(LpSolit)
Assignee

Comment 6

8 years ago
This was most likely regressed by bug 585802, although in a sense, it only exposed a bug in confirm-user-match.
Depends on: 585802
Keywords: regression
Assignee

Comment 7

8 years ago
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

8 years ago
(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

8 years ago
Comment on attachment 512478 [details] [diff] [review]
v1

Works fine. Thanks. r=LpSolit
Attachment #512478 - Flags: review?(LpSolit) → review+

Updated

8 years ago
Flags: approval4.0+
Flags: approval+
Assignee

Comment 10

8 years ago
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.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED

Updated

8 years ago
Flags: testcase? → testcase+

Updated

7 years ago
Blocks: 756314
You need to log in before you can comment on or make changes to this bug.