don't validate JS source comments



4 years ago
3 years ago


(Reporter: davemgarrett, Unassigned)





4 years ago
I just ran the AMO addon validator on a test build and got this warning:

Potentially unsafe preference branch referenced
Signing severity: low
Warning: Changing network preferences may be dangerous, and often leads to performance costs.

1328 // If "network.proxy.socks_remote_dns" is set to true or the proxy is otherwise set up to be the one to do all DNS resolution, then don't do ...
1329 if ( (proxyinfo != null) && (proxyinfo.flags &

Yeah... it's giving me a warning because I mentioned the existence of a network preference in a source comment. :/

Please do a little bit to reduce the false-positives this thing gets by not validating the comments. (at least the "//" single line comments, if multi line comments using "/*" & "*/" are hard to parse)
:kmag, how feasible do you think this is?
Flags: needinfo?(kmaglione+bmo)
Most of our tests use parse tree analysis, so they aren't triggered by code in comments. Some tests use regular expressions, which unfortunately are not able to distinguish comments from code. Unfortunately, the grammar of JavaScript is complicated enough that we really can't reliably filter out comments without going through full lexical analysis, which isn't really doable.

As for this particular warning, it should go away when I move preference tests to the regex test class which only checks the values of JS strings.
Last Resolved: 4 years ago
Flags: needinfo?(kmaglione+bmo)
Resolution: --- → INVALID
Product: → Graveyard
You need to log in before you can comment on or make changes to this bug.