1173090 - false-positive "dangerous preference" warnings on pref set under "extensions.add-on-name." branch

Status: RESOLVED INCOMPLETE

(addons.mozilla.org Graveyard :: Add-on Validation, defect)

Description

[Dave Garrett]

AMO addon validator scan for Flagfox 5.1:

https://addons.mozilla.org/en-US/developers/upload/dd852c92cb31454abeef280243f9cb35

==================================================
Attempt to set a dangerous preference

Warning: Extensions should not alter preferences outside of the 'extensions.' preference branch. Please make sure that all of your extension's preferences are prefixed with 'extensions.add-on-name.', where 'add-on-name' is a distinct string unique to and indicative of your add-on.

chrome/flagfox/content/options.xul

function setShowFavicons()
{
    Services.prefs.setBoolPref("extensions.flagfox.showfavicons",document.getElementById("showFaviconsCheckbox").checked);
==================================================
Attempt to set a dangerous preference

Warning: Extensions should not alter preferences outside of the 'extensions.' preference branch. Please make sure that all of your extension's preferences are prefixed with 'extensions.add-on-name.', where 'add-on-name' is a distinct string unique to and indicative of your add-on.

chrome/flagfox/content/options.xul

{
    var menulist = document.getElementById("openLinksInMenu");
    Services.prefs.setCharPref("extensions.flagfox.openlinksin", 
==================================================

Uh... huh?

I converted all of Flagfox's pref names from "flagfox.*" to "extensions.flagfox.*" in Flagfox 5.0 in February 2014. Those quoted lines in those warnings are quite clearly setting prefs on the "extensions.add-on-name." branch, as instructed. There's a third warning of this type as well, but that's a more legitimate false-positive because the pref name is in a var that the validator can't see easily. For these two above... I have no idea; the validator just looks broken. :|

Comment 1

[Dave Garrett]

Actually, I think I see where the validator is getting confused. Here's a link to the scan from the file upload for publish:
https://addons.mozilla.org/en-US/developers/addon/flagfox/file/321872/validation

The link in comment 0 was to a test upload. I did early scans to write out the notes to the editor before uploading.

It looks like on a regular upload for a test it doesn't scan for the addon name, but does know it on upload for publish. If that's the case, this is more minor, but still should be fixed for consistency, or at minimum say what it's complaining about.

Comment 2

[Kris Maglione [:kmag]]

Sorry, I'm not sure what you're saying in comment 1.

I can't really say anything more about this unless I see the actual file that resulted in the first output in comment 0.

Comment 3

[Dave Garrett]

It's the exact same file. What I'm saying in comment 1 is that it appears that the validator gives different results when you upload an addon file to AMO for editor review and publish, vs. when you just upload it for a standalone test here:
https://addons.mozilla.org/en-US/developers/addon/validate

Side note: Might be useful to stick a SHA256 hash of the file in the validator output so you can see when the files from two scans are the same.

The file is currently sitting in the review queue here:
https://addons.mozilla.org/firefox/downloads/file/321872/flagfox-5.1.0-sm+fx.xpi?src=devhub
(I just uploaded it earlier today)

That said, my hypothesis appears to be be wrong. I just uploaded the same file to the standalone tester a couple more times, and did not get these warnings this time:
https://addons.mozilla.org/en-US/developers/upload/360fdefd113e4566a5890158a6f13c13
https://addons.mozilla.org/en-US/developers/upload/348cfa7e74cc4794a448ac2a70651a00

Weird.

Sorry, but I guess this is isn't easy to reproduce. I currently have just what's in comment 0 for you, then. :(

Comment 4

[Dave Garrett]

If you definitely don't have enough to go on here, you may just need to close this as WFM due to lack of reproducibility.

Comment 5

[Kris Maglione [:kmag]]

Closing as incomplete. Please reopen if you see this again.