<biesi> myk: could you take a look at bug 102848, looks like bugzilla did soemthing wrong there... <biesi> myk: the patch had review+ from ducarroz, I wanted to request superreview from bienvenu... <biesi> myk: also, the process_bug page told me that no mail was sent when I added the superreview? status, shouldn't at least the requestee have got mail? <biesi> myk: in addition, bugzilla mailed me: "Christian Biesinger <cbiesinger@web.de> has approved your request for review" wtf? additionally, the requests.cgi page now shows two requests for the same status on the same attachment.

*** Bug 179495 has been marked as a duplicate of this bug. ***

ah yeah, maybe I should mention the actual issue here... the attachment flag box looks like this: cbiesinger: superreview? (bienvenu) cbiesinger: superreview? (bienvenu) cbiesinger: review+ I would have expected only one superreview? and the review+ should be ducarroz's not mine.

This patch fixes the problem, but it may not fix it all the way, and I think it needs to be carefully scrutinized.

Comment on attachment 105884 [details] [diff] [review] patch v1: fixes problem The logic here looks right. Skip if: - the status is the same, and - there couldn't have been a user; or - there was originally a user, and tha tuser is still the same r=bbaetz

a=justdave

>a=justdave myk goes to all the trouble to upgrade Bugzilla, and for what?

this also fixes an issue where if you edit an attachment with a set flag and submit without changing anything, it sends an additional email about the flag being set.

that fixes it? great... earlier today, I had experienced that bug on bmo

..except that myk told me that this patch was already on bmo...

Actually, theres another problem - bug 179643 is a separate issue. Going to comment there now

No, forget that - it is the same issue

This patch deals with additional cases of this bug that have been reported since the first patch was applied to b.m.o Monday morning. This patch has also been applied to b.m.o. The patch fixes the problem by trimming the email address field, which is given a space on the end by the user matching code in User::match_field.

cc:ing Erik for explanation of User::match_field's behavior. Erik, why does User::match_field add a space to an email address for a single-user field?

*** Bug 179643 has been marked as a duplicate of this bug. ***

re: comment 13 > Erik, why does User::match_field add a space to an email address for a > single-user field? It was a brain-dead delimiter being tacked on to everything, which worked in everything but RT. This patch should fix it.

Comment on attachment 106392 [details] [diff] [review] Fixes the appended-space problem Rather than testing the length, just use the string directly - its quicker, since you just want to know if the string is empty or not

re: comment 16 > Rather than testing the length, just use the string directly - its quicker, > since you just want to know if the string is empty or not I guess I get into the habit of testing the length because the string could possibly contain something like '0', especially if logins stop being email addresses. Boolean test against the string doesn't just fail if it's empty. Anyway, this patch does the boolean string test instead.

Comment on attachment 106437 [details] [diff] [review] Fixes appended-space problem (new) This looks fine, r=bbaetz. Can myk check that this solves teh other problem w/o needing this workarround? It should, but....

This patch combines not_erik's and my approaches, since each fixes only part of the problem. not_erik's fix deals with the general case, while my fix targets specific kinds of transactions for which the earlier logic (for determining whether or not the flags had changed) was faulty.

Comment on attachment 106559 [details] [diff] [review] patch v5: superset of fixes r=bbaetz, but change the &::trim to trim, and |use Bugzilla::Util;| if you need to include that to get this working

Checking in Bugzilla/User.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/User.pm,v <-- User.pm new revision: 1.9; previous revision: 1.8 done Checking in Bugzilla/Flag.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Flag.pm,v <-- Flag.pm new revision: 1.6; previous revision: 1.5 done

Somehow I missed bbaetz' last comment when I checked this in. This patch fixes that.

Checking in Bugzilla/Flag.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Flag.pm,v <-- Flag.pm new revision: 1.7; previous revision: 1.6 done