Add-ons/Reviewers/Guide/Reviewing: Difference between revisions

= Step 3: Code Review  =

All add-on code must be reviewed. The code validator can't detect all possible security or code quality issues, which is why we have a review team.

All Add-on History entries have a View Contents link that take you to the code browser page. Updates also have a Compare link, which will show you the code with the changed sections highlighted. For updates, the compare link should be used. Validation results are integrated into the code viewer, so you can see validation warnings in context. It's okay if you prefer to use other tools for code analysis and diffing.

== Libraries, frameworks and other unreadable code ==

It's very common for add-ons to use libraries or frameworks such as jQuery or Bootstrap. Some add-ons use complex frameworks like Kango, usually to achieve cross-browser compatibility. Finally, the Add-ons SDK generates various files around the actual add-on code. Our code validator will try to detect them. If detected, library code won't generate other validator messages, and it is greyed out in the code viewer.

 

If a library or framework isn't detected, it can mean that it was either modified by the developer (and the add-on should be rejected), or there's a problem in our detection code, in which case it should be brought up on the mailing list. Library or frameork code that has been detected as valid doesn't need to be reviewed.

 

Aside from libraries, many add-ons can include minified, obfuscated or compiled code. Since this code can't be easily reviewed without the original sources, only admin reviewers can review them. The Add-on History entry should indicate if the source code has been provided by the developer. If that's not the case, you can use the canned info request.

{| width="80%" cellspacing="0" cellpadding="1" border="0"

! style="border-bottom: 2px solid black" scope="col" | Issue

| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Remote code download or execution, custom update code.

| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | As explained in the validation section, no remote code execution is allowed.

| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Using plain HTTP for security-sensitive operations.

| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Sending passwords over unprotected HTTP if there's a secure alternative. Sending passwords in the URL or other headers than POSTDATA. Performing any security-sensitive operations over HTTP when there's a secure alternative.  

| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Using non-release version of Add-ons SDK

| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary review

| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | The validator can detect many experimental versions of the SDK, but that doesn't make it OK to use in public add-ons. Public add-ons should use release versions of the SDK. Others should only get preliminary review.

| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary review

| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | We accept the two most recent minor releases of the Add-on SDK. If the validator warns that the add-on is using an outdated SDK version, it is acceptable for full review if it is one release older than the suggested release. Otherwise, it is only eligible for preliminary review.

| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary review

| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preference names without "extensions." or "services.sync.extensions." prefix  

| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary review (nominations only)

| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Add-on preferences should use the "extensions." prefix, and should also have a reasonable namespace (like "extensions.myVideoDownloader."). If the add-on is already public, this rule should be ignored.

| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary review

| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | An add-on can claim to work with a popular website like Twitter, but then send the user data through some other site, most likely owned by the developer. There needs to be a justified reason to handle user data in this manner, and the privacy policy and add-on descriptions need to be very clear about this. Passwords should never be handled in this way, and they should only be transmitted directly to the original API provider (Reject otherwise).

| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary review

| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Synchronous (non-local) HTTP requests, synchronous SQL queries, noticeably inefficient code, random UI freezes, loading large amounts of JS code directly in overlays.

| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Some add-ons need helper files like SQL DBs or logs. Those files must be written in the profile folder, not the extension installation folder or other locations.

| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Some things to look for: the add-on must not require a restart for any of its features to fully work, must not require a restart after being disabled or uninstalled, must unregister all observers and remove all UI when disabled or uninstalled. [[http://maglione-k.users.sourceforge.net/bootstrapped.xhtml More details].

| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | If the author keeps submitting the same version, contact the mailing list.

| style="padding: 0.5ex 1ex 1ex 0pt; border-bottom: 1px solid black;" | Using non-standard preferences UI

| style="padding: 0.5ex 1ex 1ex 0pt; border-bottom: 1px solid black;" | Add note

| style="padding: 0.5ex 1ex 1ex 0pt; border-bottom: 1px solid black;" | If an add-on has a preferences UI, it should use one of the [https://developer.mozilla.org/en-US/Add-ons/Install_Manifests#optionsURL supported methods].

| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Add note

If the add-on version is not nominated for full review, or if by this point you have sufficient information to deny the add-on full review, you can stop reviewing and submit what you have.