332452 - Can't approve extension without checking "New Chrome" checkbox

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Administration, defect)

Description

[:Gijs (he/him)]

Language packs such as https://addons.mozilla.org/extensions/moreinfo.php?id=517 don't add new chrome. I want to be able to approve it without checking that checkbox. If I can only approve when checking all those checkboxes, then why are they there at all?

Similarly, I always thought I shouldn't check the "clean profile" if I didn't use a clean profile to check it (which would technically speaking also be the case here - the extension needs chatzilla installed, and it's not exactly the only extension that can't live on its own).

To fix, alter:
http://lxr.mozilla.org/update1.0/source/developers/approval.php#73

If it's wanted, I can provide a patch. Please poke me on IRC if necessary, or comment here.

Comment 1

[Cameron]

Saw your discussion on IRC when I woke up today. I find it hard to believe you wouldn't search for other bugs, but isn't this a dupe of bug 278771?

Comment 2

[:Gijs (he/him)]

I did search, but that bug's summary says fuck-all about the actual problem described, imho. It's about as general as it could have been, making it hard to find. I searched for "chrome" or "checkbox" (in summary), none of which popped up this bug.

I also seem to read that that bug is more of a "I'm not happy with checking all those checkboxes, it's not necessary" than something complaining about the semantics of these checkboxes, ie, there are occasions when it is not *correct* to check all the checkboxes, very much apart from the fact that it's simply a pain in the ... to do it every time. I try to follow the 'policy' (which barely exists anyway, as proven by this bug - surely I'm not the first to review an extension where checking either of these checkboxes is not semantically correct?), and that in turn proves inpossible because of limitations in the system. I'm adding a dependency for now, if the powers-that-be think I'm wrong, they're free to dupe with an explanation as to why they think I'm wrong.

Comment 3

[alanjstr]

Well, you can think of it this way.  You check to box to indicate that you verified there's no chrome change.

Comment 4

[:Gijs (he/him)]

(In reply to comment #3)
> Well, you can think of it this way.  You check to box to indicate that you
> verified there's no chrome change.

Huh? So for one extension, I check it because they add new chrome, and for another because there is none? Then why is the checkbox there? It is semantically useless if I understand your suggestion correctly.

Comment 5

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment #3 is not a good way to "think of it", it's a rationalization for a flat-out bug!  We should at least fix this in 2.1, even if we won't get the newly CRUDdy developer/reviewer/admin area.

Comment 6

[Christopher Finke]

I agree with Mike ang Gijs; the checkbox appears to exist for the purpose of relaying whether the extension added new chrome.  Checked if it does, unchecked if it doesn't, given that the tooltip is in question form ("Extension added chrome to the UI?") and not a confirmation of checking for chrome ("Performed check for new chrome added to the UI.")

So either the checkbox title/tooltip should be changed to reflect that it should always be checked for an approval, or the requirement that it must be checked should be dropped.

Comment 7

[Justin Scott [:fligtar]]

No checkboxes in Remora.

(and there was much rejoicing)