1349769 - takebug should only be default for patch attachments

Status: VERIFIED FIXED

(bugzilla.mozilla.org :: General, defect, P1)

Description

[Daniel Holbert [:dholbert]]

STR:
 1. Visit some bug which is unassigned -- e.g. at the moment, try visiting
 https://bugzilla.mozilla.org/show_bug.cgi?id=1332359

 2. Click "Attach File"

 3. Scroll down and look for the "take bug" checkbox.

EXPECTED RESULTS:
The checkbox should not be checked.

ACTUAL RESULT:
The checkbox is checked by default (and presumably you'll assign the bug to yourself, by accident, if you complete the form without noticing.)


I suspect this must be an simple mistake, though perhaps we made this change intentionally, based on the assumption that posting an attachment means you're taking the bug...?  If so, I very much dispute any such assumption.  Posting an attachment is absolutely *not* an expression of intent to take a bug.  When a bug is unassigned, attachments are more likely (or at least quite likely) to be testcases/logs/screenshots from someone reporting/confirming the bug, and these actions are not an expression of intent to assign the bug to themselves.

Comment 1

[Daniel Holbert [:dholbert]]

I believe this started happening pretty recently -- I ran into it yesterday, and I don't recall running into it before then.

(Also, for now at least [while this bug is unassigned], you can reproduce the bug [default-checked checkbox] using the "Attach file" link on this very bug report here.)

Comment 2

[Daniel Holbert [:dholbert]]

(Hoping Dylan can take a look or farm this out to somebody.)

Comment 3

[Dylan Hardison [:dylan] (he/him)]

Sorry, this was a recent change. We can back it out next week. Possibly may it only apply when it's a patch?

Comment 4

[Dylan Hardison [:dylan] (he/him)]

Attachment: 1349769_1.patch

I think the gap in my understanding here for bug 1344979 was how many unassigned bugs get non-patch attachments. Here's a patch (hah) that only checks "take" when the attachment is a patch. Along the way it removes some onchange= style event handlers and even adds [% csp_nonce FILTER none %] which will be an empty string currently, but might as well add it now.

Comment 5

[Dylan Hardison [:dylan] (he/him)]

Bah, with the minor nit that s/csp_nonce/script_nonce/. Note that is actually a problem itself, as script_nonce will look wrong when we start using it on <style> tags. Will fix on commit assuming an r+

Comment 6

[:glob ✱]

Comment on attachment 8850277 [details] [diff] [review]
1349769_1.patch

Review of attachment 8850277 [details] [diff] [review]:
-----------------------------------------------------------------

r=glob lgtm

Comment 7

[Dylan Hardison [:dylan] (he/him)]

To [e-mail]:mozilla-bteam/bmo.git
   dc6eca0..9af56b9  master -> master

Comment 9

[Dylan Hardison [:dylan] (he/him)]

hmm, when I tested this locally, it worked when I attached a patch and the patch pre-checked "patch" for me, but this doesn't work on production. It works if patch is manually chosen. I'll follow up after some other things. 

At least this won't be annoying anyone