575452 - The new starring UI can cause some bugs which were starred previously to be commented on again when starring new bugs

Status: RESOLVED FIXED

(Tree Management Graveyard :: TBPL, defect)

Description

[(no longer active)]

This was discovered by philor.

What happens is that when you have star some bugs by clicking the star icon next to the suggestion list, and a tinderbox data load finishes in between, sometimes the code leaves some __active_123456 items around, somehow.

I'm not yet sure how exactly that happens, but I have a patch which should fix any similar issues.  Instead of using the __active_123456 class name hack, I switched to using a normal object, which is cleared when needed (for example, when clicking another log, or after submitting comments.)

Comment 1

[(no longer active)]

Attachment: Patch (v1)

Comment 2

[(no longer active)]

Markus, since this is a serious issue, would you mind landing and deploying it if you r+ it (or if you want to make some changes before pushing)?

Comment 3

[Markus Stange [:mstange]]

http://hg.mozilla.org/users/mstange_themasta.com/tinderboxpushlog/rev/2a28e0beda5d

Comment 4

[Phil Ringnalda (:philor)]

Crap. I haven't figured out what the essential step is to make it happen (other than maybe "use my fairly slow connection"), but this is still happening.

Comment 5

[Phil Ringnalda (:philor)]

Maybe not my connection, since Ted just hit it, too.

Comment 6

[Phil Ringnalda (:philor)]

And karl.

I suspect from an IRC conversation that the only reason more people aren't hitting it is that hardly anyone knows that the star icon in front of the text "add a comment" link actually does something different than the text link.

Since it's apparently pretty widely busted, maybe we should remove the icon, just keeping the stars in the suggestions as a way to preselect bugs in the dialog? That part works just fine.

Comment 7

[Steve Fink [:sfink] [:s:]]

Attachment: Reset active things in submit member because submit handler never gets called

Aha! This is where the strange _autoStarBugs thing came from. I couldn't figure out what it was for, and mostly removed it in bug 657690.

This bug happens because we are *NOT* clearing out the set of active bugs on a magic-bottom-left-star submit. I'm guessing that the confusion is that there's a jquery binding on the submit action that should reset everything, but it's not getting invoked in the commentWithoutUI() case since commentWithoutUI "submits" the form by calling self.submit() directly where self=AddCommentUI. There's no event to handle.

Fix attached.

Comment 8

[(no longer active)]

Comment on attachment 533554 [details] [diff] [review]
Reset active things in submit member because submit handler never gets called

Oh, I totally should've caught that!  Sorry, and thanks for picking up my slack :-)

Comment 9

[Steve Fink [:sfink] [:s:]]

http://hg.mozilla.org/users/mstange_themasta.com/tinderboxpushlog/rev/a4c1253517cc