345958 - Do not throw an error if an inactive flag type is set to "X" while editing a bug

Status: RESOLVED FIXED

(Bugzilla :: Attachments & Requests, defect)

Description

[timeless]

Inactive Flag Types

Bugzilla has suffered an internal error. Please save this page and send it to [e-mail] with details of what you were doing at the time this message appeared.

URL: https://bugzilla.mozilla.org/process_bug.cgi?delta_ts=2006-07-25+03%3A37%3A30&longdesclength=4&id=345810&alias=&product=Core&component=XP+Toolkit%2FWidgets%3A+Trees&rep_platform=PC&op_sys=Windows+XP&version=Trunk&priority=--&bug_severity=critical&target_milestone=---&newcc=&qa_contact=xptoolkit.trees%40core.bugs&bug_file_loc=&short_desc=Thunderbird+crashes+at+startup+in+%5B%40nsTreeBodyFrame%3A%3AGetIndentation%5D&status_whiteboard=&keywords=crash&flag_type-206=X&flag_type-192=X&flag_type-233=X&flag_type-537=X&flag_type-164=X&flag_type-221=X&flag_type-207=X&flag_type-97=X&dependson=&blocked=&comment=&knob=none&resolution=FIXED&dup_id=&assigned_to=Jan.Varga%40gmail.com&form_name=process_bug
Some flag types are inactive and cannot be used to create new flags.

This instance is contrived, i randomly corrupted an =X flag to be some random number.

In short: any =X flag should be ignored from whatever silly strict checking we're using.

The actual steps involve loading a page w/ a flag unset and waiting for an admin to twiddle the flag, then committing. usually the victim has written a long comment.

Comment 1

[Jesse Ruderman]

This happened to at least me and reed just now when timeless made the flag for blocking Firefox 1.5.0.5 obsolete.

There are two problems here, IMO:

(1) If I leave a flag set to empty (X), e.g. when I'm doing nothing other than adding a comment, Bugzilla shouldn't complain as if I'm trying to nominate an obsolete flag.

(2) If I *do* try to nominate an obsolete flag, it shouldn't be a fatal error; Bugzilla should just warn me and submit the rest of my changes normally.

Comment 2

[Frédéric Buclin]

(In reply to comment #1)
> (1) If I leave a flag set to empty (X), e.g. when I'm doing nothing other than
> adding a comment, Bugzilla shouldn't complain as if I'm trying to nominate an
> obsolete flag.

I agree.


> (2) If I *do* try to nominate an obsolete flag, it shouldn't be a fatal error;
> Bugzilla should just warn me and submit the rest of my changes normally.

I disagree if other changes suppose the flag to be set. Why shouldn't we throw an error for flags while we throw errors when changing a field you are not allowed to change? I will think about this second point a bit.

Comment 3

[Max Kanat-Alexander]

I agree with LpSolit about point #2. Bugzilla should let you know if it's not going to do exactly what you told it to do.

Comment 4

[Frédéric Buclin]

Attachment: patch for tip, v1

Comment 5

[Frédéric Buclin]

Attachment: patch for 2.22, v1

Reviews are required on branches, even for module owners...

Comment 6

[Myk Melez [:myk] [@mykmelez]]

> I disagree if other changes suppose the flag to be set. Why shouldn't we 
> throw an error for flags while we throw errors when changing a field you are
> not allowed to change? I will think about this second point a bit.

Because most of the time the rest of the changes are still valid, so it creates much less extra work to let them through than to reject them.


> I agree with LpSolit about point #2. Bugzilla should let you know if it's not
> going to do exactly what you told it to do.

I agree that Bugzilla should let you know about the situation, but I disagree that it should also reject the rest of your changes.  At most, it should tell you about the invalid flag and ask you to confirm that you still want to submit your other changes.

Comment 7

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 235299 [details] [diff] [review]
patch for 2.22, v1

This fixes Jesse's first issue, which we all agree about. r=myk

Comment 8

[Frédéric Buclin]

tip:

Checking in Bugzilla/Flag.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Flag.pm,v  <--  Flag.pm
new revision: 1.76; previous revision: 1.75
done
Checking in template/en/default/global/code-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/code-error.html.tmpl,v  <--  code-error.html.tmpl
new revision: 1.78; previous revision: 1.77
done


2.22:

Checking in Bugzilla/FlagType.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/FlagType.pm,v  <--  FlagType.pm
new revision: 1.23.2.2; previous revision: 1.23.2.1
done
Checking in template/en/default/global/code-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/code-error.html.tmpl,v  <--  code-error.html.tmpl
new revision: 1.62.2.3; previous revision: 1.62.2.2
done