31117 - should have a "submit only my comments" option on midair collision

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, enhancement, P2)

Description

[Dave Miller [:justdave]]

Some suggestions for the midair collision warning:

Since it states right in the "submit anyway" text that added comments will not be 
overridden, the midair collision warning should not happen if the other thing the 
previous person to edit the bug did was add a comment.  Since it displays the 
diff right on the page, it's pretty obvious that was the only thing done, and the 
collision detection should be smart enough to figure that out and not give the 
warning.

Conversly, if the CURRENT edit is only adding a comment, this should also 
automatically happen, and not issue the warning.  (on the other hand, it could be 
a duplicate post from the same browser, which is probably also detectable, and 
the warning should occur in this case).

Also, instead of only "Submit anyway and erase the other persons changes (except 
comments)" and "throw out my changes and revisit the bug", it should also have a 
choice to "add my comment anyway, but ignore any other changes I made".

Comment 1

[Matthew Tuck [:CodeMachine]]

I agree.  Also, non-interfering clashes should be able to go through
independently, but the second submitter should still get a warning first asking
whether to cancel, overwrite the first change or occur as well as the first
change.

Whether either of these is feasible is another matter of course.

Comment 2

[Terry Weissman]

I disagree on all counts.

The thing is, if someone else has changed the bug, then your changes may no
longer make sense.  Someone else may have added a comment that answers the
question your comment was going to ask.  In that case, you don't want to go
ahead and ask your question; that would just make you look stupid.

Similarly, if someone changed fields and you are changing a different set of
fields, it still may not make sense for your change to go through.

I am not going to try and do any kind of intelligent merge.  It works out to
rocket science, and it also is not guaranteed to be right.

I am happy with the way it works right now.

Comment 3

[Dave Miller [:justdave]]

OK, I will concede to that for the reasons you stated, however, I still think 

there should be a choice there for "apply my comment anyway, but ignore the rest 

of the changes I made" on the midair collision warning.  It would probably also 

help to include some text on that warning indicating that the user may have 

caused it himself if they used the back button to get to the bug after already 

submitting changes to it.

Comment 4

[Matthew Tuck [:CodeMachine]]

Terry, as I said, the second submitter could have an option to do an independent
change on the clash warning along with the replace and cancel options.  I guess
this would apply to the comments as well.

I presume apply only comment and apply only actions might be additional options
but I don't think they're needed.

Comment 5

[Matthew Tuck [:CodeMachine]]

Terry, any reason not to reopen given the comments above?

Comment 6

[Dave Miller [:justdave]]

We can always have Tara give it a fresh look.  This was already closed when she 

took over, so it didn't get moved to her.  Since I reported it, I think it'll let 

me reopen it, I will do so and reassign it to Tara.

Comment 7

[Dave Miller [:justdave]]

reassigning to current component owner...

Comment 8

[Chris Yeh]

i'm siding more with terry on this one: doing intelligent merges is rocket 
science, and if the bug has changed underneath you, you should go see what the 
heck happened. 

Comment 9

[Matthew Tuck [:CodeMachine]]

As far as I know the changes are all listed on the page.  If they aren't, they
should be before this is implemented.

Comment 10

[Dave Miller [:justdave]]

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

Comment 11

[Dawn Endico]

Adding brendan and jst to cc who were on bug marked as dup of this.
I think I agree with Terry's 2000-03-09 09:49 comment that this is a bad idea.
Your real problem is Mozilla's back button horkage.

Comment 12

[Brendan Eich [:brendan]]

I agree with Terry, and withdraw my extravagant claim.  But the message about
"except for changes to the long description" is confusing, or unclear, or not
noticeable enough, or something -- that led jst (among others) to fear
submitting his comment-only change anyway, and he then went further astray out
of (rational) fear of evil Mozilla cut-paste and back button bugs.

I've reopened and morphed bug 70039 into what it should have been, a request for
a clearer exception message.

/be

Comment 13

[Stephan Niemz]

moving to real milestones...

Comment 14

[Jesse Ruderman]

See also bug 53452, "[RFE] Mid-air collision: let me copy my comment out of a 
textarea", for working around the problem that some browsers doesn't restore 
form data when you press "back".

Comment 15

[Andreas Franke (gone)]

-> Bugzilla product, Changing Bugs component, reassigning.

This bug should probably be resolved (e.g. as WONTFIX or INVALID).

Comment 16

[Jacob Steenhagen]

Rather than marking WONTFIX, I think this bug should be morphed to be what
Dave's 2000-03-09 09:57 comment was.

Doing just that :)

BTW, the wording on the mid-air page has already been fixed.

Comment 17

[Dave Miller [:justdave]]

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

Comment 18

[Jason Bassford]

I'd been meaning to open a bug on the confusing state of affairs of mid-air
collisions for a long time now.  I just had it happen to me again and, this
time, I took the opportunity to check around.

I have several comments:

1) Despite bug 70039, the mid-air collision warning is STILL quite vague and
confusing.  Unless you pay *very* close attention to the wording, it's not
immediately obvious what the results of submitting changes will be.  (I'm still
not ENTIRELY sure what will happen in any given circumstance - see 3) below.)

2. Almost every time I see a mid-air collision, the resolution that I would like
to have happen is for my own comments to be added without changing anything
else.  Hence, I very strongly support what Dave suggests in comment 3.  There
needs to be a way of simply appending your own comments, and discarding any
other non-comment changes you might have made.

3. Say a previous change was to mark a bug a dupe and to add some comments, and
 my change is just to add a comment, nothing more.  What happens if I over-ride?
 Does the bug get un-duped?  (Very unclear text as to what will happen here.) 
It seems to me that it should only be a matter of "overriding" when two changes
are diametrically opposed.  (For example, I dupe to one bug, while the other
person has duped to a different one.  Or, I resolve it as WONTFIX while the
other resolves it as INVALID.)  If there is nothing about the one submission
that is opposed to the other submission, the changes should be additive.  (I.e.
I change the OS, while the other person adds a keyword - the two should just be
merged.)

Regardless of what actually does happen, the warning message does not
sufficiently convey what will happen.

4. The warning page does NOT explicitly list every change that was made.  The
one I just got says that the only change made was the comment "*** Bug 190332
has been marked as a duplicate of this bug. ***"  However, that's not true.  In
addition to that automatically generated comment, the bug WAS marked as a
duplicate of another.  So, really, you should be seeing *two* changes listed. 
The added commment, and the dupe.  This may be redundant, but I don't think that
the comment, in and of itself, is sufficient.  (We should never assume.)  What
if, for instance, I were to add a comment to the bug using that exact text (and
not actually resolve it as a dupe).  In such a case, the bug would not have been
duped, yet the warning the mid-air collision gives would be identical to the
situation in which it actually was.  The person reading the warning would not be
aware that it was only an additional comment.

Comment 19

[Jason Pyeron]

see bug 53452, comment 8 where I created a patch [attachment (id=125189)]

this should appease those who want to not loose their comments.

but I definatly agree with comment 2

Comment 20

[Scott Steesy]

I agree with comment #3 ... solely because I just had a problem submitting a comment on a bug for cPanel on their site which uses bugzilla ... I can't tell which version.

All I did was enter a new comment and click submit.  It told me I had changed "OS from to All" and that I wasn't the bug owner ... so wouldn't allow me to save my comment at all.  Note that it appearantly had nothing for the OS on that bug, and the combobox for OS defaulted to All so it thought I had changed it when I hadn't.

Comment 21

[Wayne Mery (:wsmwk)]

I submit this should be the default action.

how does this depend on bug 107306? 107306 makes the solution for this bug to be far more complex than necessary.

Comment 22

[Miles]

Not certain this is the correct bug for this comment:  I started to send a comment to a bug.  Immediately realized an additional statement was needed, so hit Escape.  Added the additional comment and committed by pressing -- what did it say to the effect, overwrite the prior comment and add this one.  Now both comments are seen, the 2nd being a copy of the 1st however with the additional statement.   (And this is not a one-time shot for it occurred once prior for another bug.) 

Comment 23

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

That's unrelated to this bug.

Comment 24

[David Lawrence [:dkl]]

Attachment: Patch to allow submitting comment anyway during midair collision (v1)

Here is the patch we currently use that allows for submitting comment only in the event of a midair collision. This doesn't take into account any other changes that were made but does allow the user to submit comment if that was the only change they made.

https://bugzilla.redhat.com/show_bug.cgi?id=150022

Thanks
Dave

Comment 25

[Frédéric Buclin]

Comment on attachment 353227 [details] [diff] [review]
Patch to allow submitting comment anyway during midair collision (v1)

>+               value="[% bug.delta_ts FILTER html %]"/>

No need to write />, we don't it elsewhere; just write >. This is HTML4, not XML. :)


>+        <input type="submit" id="process" value="Submit only my new comment"/>

This ID is already used in the previous form. You must choose another one. Maybe comment_process?


I didn't test your patch yet, but it looks good.

Comment 26

[Frédéric Buclin]

Note to self: I will have to update bug 26257 if this patch lands before the security bug.

Comment 27

[David Lawrence [:dkl]]

Attachment: Patch to allow submitting comment anyway during midair collision (v2)

Thanks. I have attached updated patch with your suggested changes.

Dave

Comment 28

[Frédéric Buclin]

Comment on attachment 353460 [details] [diff] [review]
Patch to allow submitting comment anyway during midair collision (v2)

Works fine. r=LpSolit

Maybe some padding between buttons would be good, but this is a minor UI improvement which can be fixed later, if we want to.

Comment 29

[David Lawrence [:dkl]]

tip:
Checking in template/en/default/bug/process/midair.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/process/midair.html.tmpl,v  <--  midair.html.tmpl
new revision: 1.23; previous revision: 1.22
done

Comment 30

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Yay! Thanks for fixing this, it'll take care of 90% of my midair collisions.

Comment 31

[Max Kanat-Alexander]

Added to the release notes for Bugzilla 3.4 in bug 494037.