Canmove, confirm
1,448
edits
Changes
WIP
== Performing a Review ==
'''Reviewer Intro Tour:''' ask your guide to select an add-on for you to review. Don't submit your first review without their pre-approval!
Add-on reviewers have a great responsibility. We need to ensure add-ons are safe to use, good quality, and clearly presented to our users. We also need to make sure developers get quick, clear, and actionable reviews.
AMO is designed so that popular and well-reviewed add-ons bubble to the top search rankings. The full and preliminary review levels also create two layers that allow us to list add-ons that are not quite ready for general consumption. Since we have good filtering mechanisms on AMO, our general policy is to '''only reject when strictly necessary'''. Rejection is necessary when an add-on has security issues, doesn't meet our content policies, or a few other cases which are spelled out in this guide. If an add-on doesn't meet the criteria for rejection, it should at least get preliminary approval.
=== Step 1: Review Add-on Metadata ===
Before getting started, here's a very important legal note: reviews '''must not''' involve checking for copyright or trademark violations. You should not take '''any''' action on an add-on because you suspect it copies code from others without permission or may otherwise infringe somebody else's copyright or trademark. The DMCA is a law that gives us legal protection from being held responsible for copyright infringement by users who post content to our site, but only if our conduct qualifies us for such protection and we follow exactly the procedures laid out in the DMCA. Determining copyright or trademark infringement is complicated and you will not have enough information to make those determinations.
If you have any concerns about the legality or legitimacy of an add-on, please email amo-editors AT mozilla DOT org.
==== Policies and Actions ====
{| cellspacing="0" cellpadding="1" border="0" style="width: 80%"
|+
|-
! style="border-bottom: 2px solid black" scope="col" | Policy
! style="border-bottom: 2px solid black" scope="col" | Action
! style="border-bottom: 2px solid black" scope="col" | Notes
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Empty name (the name is blank in default locale)
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Request More Information
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject if the name is not updated after 3 days.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Missing information in descriptions, missing testing information
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Request More Information
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Add-on name and code copied from very popular add-on (like Firebug or AdBlock Plus)
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Admin Review / Notify mailing list
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | These add-ons can include some form of malicious code and trick users into thinking they are the original one.
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Add-on name or icon identical or very similar to existing, active listing
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Admin Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | We want to avoid user confusion when selecting which add-on to install. It's OK to fork, but it should be clear what are the differences between very similar offerings.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Other copyright suspicions
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Ignore
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | We have a legal obligation *not* to take action unless the author of the original code files a DMCA complaint. If you see any malicious intent in the copied add-on, follow the same Action as the previous policy.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Not compatible with the current versions of the application
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Add note
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | There's a canned response for this purpose. Reject if the maxVersion is lower than 4.0, unless it's a critical fix.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Missing Privacy Policy or EULA when necessary
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | These descriptions are necessary if the add-on handles user information remotely, or the user needs to agree to any terms in order to use the add-on.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Questionable add-on relevance
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Carefully read the [[#Add-on_Relevance|Add-on Relevance]] section below. This policy should only be enforced on extreme circumstances.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | New version doesn't follow previous editor requests
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | If the author keeps submitting the same version, contact the mailing list.
|}
==== Add-on Relevance ====
Add-on relevance should only matter if the add-on is obscure or difficult to review, or you think that it is too simple to be fully approved. In general you shouldn't make any assessments on add-on relevance, since we have download counts, active daily users and user reviews to rank all add-ons on AMO.
We have a very low admission threshold, but we don't want to fully publish add-ons that will get at most a couple hundred downloads and will be mostly ignored during their lifetime. In that case, the add-on should at best receive a preliminary review approval.
Add-ons that commonly fall into this category are:
*Add-ons that help access very specific webpages. For example, an add-on that makes it easier to use an internal business site or a university library.
*Add-ons targeted to a limited geographical area. For example, an add-on for a local city newspaper or radio station.
*Add-ons that only add a toolbar or menu filled with website links, similar to plain bookmarks.
*Add-ons that download video files from Flash video sites like YouTube. We already have too many add-ons like these, with little to no distinction between them. Unless the add-on provides something really innovative to video downloading, it should not get full approval.
Assessing the relevance of an add-on can be difficult. In these cases you should leave the review to somebody else or ask on the mailing list.
=== Step 2: Automatic validation ===
We have a extensive set of static tests that identify common bad practices and possible security problems with add-on code. You must always run the code validator and inspect the results when performing a review.
Each Add-on History entry has a validation link. You'll want to validate the latest version.
[[Image:Validation-link.png|center|Add-on validation link]]
Clicking on the link will take you to the validation page, where the automatic code validator will run for that version of the add-on and then the results will be displayed. It's usually best for navigation to open this link in a new tab.
==== Policies and actions ====
{| width="700" cellspacing="0" cellpadding="1" border="0"
|-
! style="border-bottom: 2px solid black" scope="col" | Policy
! style="border-bottom: 2px solid black" scope="col" | Action
! style="border-bottom: 2px solid black" scope="col" | Notes
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Missing file / Parse error / Validation error
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Obfuscated, minified or binary code
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | There's a canned response for this.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Obfuscated, minified or binary code, with original sources included in XPI or provided link
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Admin Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Using eval, Function(), setTimeout, setInterval to evaluate remote code
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Remote script injection
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | <browser> or <iframe> elements with no type
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | See canned response and [https://developer.mozilla.org/en/XUL/iframe#a-browser.type iframe documentation]. If the frame or browser is used to load chrome content, it's fine as long as that's clear from reading the source.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Inserting remote content with innerHTML
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | If it's only local content, Add a Note to fix it on future versions. There's a canned response.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Using DOM Mutation events
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Just add a note if this is a first warning and the add-on has been approved with this code before.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Conduit add-on without [https://addons.mozilla.org/en-US/firefox/user/4959120 CONDUIT-AMO] as author
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Conduit add-on with [https://addons.mozilla.org/en-US/firefox/user/4959120 CONDUIT-AMO] as author
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Admin Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | JS Library (like jQuery) included, but not in its original file. JS Library doesn't pass checksum validation.
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | SHA-256 hashes recognized by the validator are stored at http://mzl.la/amo-libs and [https://github.com/mozilla/amo-validator/blob/master/validator/testcases/static_hashes.txt]
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Unicode characters (e.g. \u0060) in JS code
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Keep in mind these can be used safely inside strings. They're just not allowed to replace characters in JS code, since they can be used to bypass the validator.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Using eval, Function(), setTimeout, setInterval to evaluate local code
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | One case that we accept is when eval is used to replace existing Firefox functions. This is very common for add-ons that change bookmarking or tabbing behavior. It is also allowed in known libraries like jQuery.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Using the codebase_principal_support preference or enablePrivilege function
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | They're deprecated.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Native object prototype extension / Using Prototype library
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Storing passwords or other sensitive user data in the preferences
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Changing Firefox preferences without user consent
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | These include: network preferences, update system preferences, homepage, User Agent string.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Changing security preferences, permissions, certificates (nsIX509CertDB)
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Admin Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Using nsIProcess
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Admin Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Using JS c-types
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Admin Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Using Geolocation
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Test
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Give Preliminary Review if the add-on doesn't ask the user for permission to use geolocation before getting geolocation data ([https://developer.mozilla.org/en-US/docs/Using_geolocation#Prompting_for_permission this is how users should be asked for permission]). Approve if it does.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Localization errors
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Ignore
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|}
See the [[AMO:Editors/EditorGuide/AddonReviews/Security|security code review]] page for more detail on the above security rejection reasons.
Most of the other validator flags are not that important, but they should still be fully read and understood. When in doubt, check the help page or ask in the mailing list.
=== Step 3: Code Review ===
Every line of add-on code must be reviewed. The code validator can't detect all possible security or code quality issues, so we must always be on the lookout for bad code. For versions in the Preliminary Review queue, the code review should just ensure the add-on's safety.
All review pages have a View Contents link that take you to the code browser page. Pending updates also have a Compare with Public Version link next to it, which will show you the code with the changed sections highlighted.
For updates, the compare link should be used. However, it can sometimes fail to work for very large add-ons. In that case, you can either review the full source code, or download the new and public files to your system and compare them using tools like [http://code.google.com/p/amo-editor-tools/source/browse/trunk/sh/xpidiff.sh xpidiff.sh] or [https://wiki.mozilla.org/AMO:Editors/WinMerge WinMerge].
Validation results are integrated into the code viewer, so you can see validation warnings in context. For Add-ons SDK extensions, known library files are shaded in grey, and their origin is stated in the popup that appears when hovering over them.
==== Policies and Actions ====
{| width="700" cellspacing="0" cellpadding="1" border="0"
|-
! style="border-bottom: 2px solid black" scope="col" | Policy
! style="border-bottom: 2px solid black" scope="col" | Action
! style="border-bottom: 2px solid black" scope="col" | Notes
|- style="vertical-align: top;"
| 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;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Add-ons should not download remote code in any way. They are free to interact with web APIs as long as all that is being transmitted is data, not code. Add-ons are allowed to insert local scripts into webpages (within reason), but are not allowed to insert remote scripts.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Modified on unverified Add-ons SDK library code
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Library files that are unrecognized will not appear in grey color.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Using non-release Add-ons SDK library
| 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 or beta versions of the SDK. Others should only get preliminary review.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Using outdated Add-ons SDK library
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Except under special circumstances, 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="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | HTTP security violations
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| 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="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Bad or no namespacing
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | All scripts that are included in the main window overlay should have proper namespacing to avoid name conflicts with other add-ons. The name should normally correspond to the add-on name in order to guarantee its uniqueness.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preference names without "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. If the add-on is already public, this rule should be ignored.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Extension preferences names in Sync without "services.sync.extensions." prefix
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Add-on preferences using the Sync service of Firefox should use the "services.sync.extensions." prefix, and should also have a reasonable namespace.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Privacy issues
| 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.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Performance problems
| 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 requests, noticeably inefficient code, random UI freezes, loading large amounts of JS code (in the order of tens of thousands of lines of code) directly in overlays.
|- style="vertical-align: top;"
| style="padding: 0.5ex 1ex 1ex 0pt; border-bottom: 1px solid black;" | Not using prefwindow for preferences
| 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 window, we recommend that authors do 2 things: 1) use the ''prefwindow'' element, 2) add the line in install.rdf that enables this window to be opened from the Add-ons Manager window.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Duplicate / hidden files or folders
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Add Note
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | See canned response.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Unpacked add-on writes files in own installation directory
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Writing in the installation directory can generate problems with the Add-ons Manager, where the add-on can be stuck in an update loop. Add-ons should write all files in the profile dir.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Bootstrapped Add-on doesn't clean up after itself
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | See section on [[AMO:Editors/EditorGuide/SpecialAddonTypes#Bootstrapped_.28Restartless.29_Add-ons|Bootstrapped Add-on policies]].
|}
See the [[AMO:Editors/EditorGuide/AddonReviews/Security|security code review]] page for more detail on the above security rejection reasons.
In general you should apply your judgement and try to identify code that may appear suspicious or out of place. Try to understand what everything does and how it all fits together.
By the end of the code review, you'll may have a series of notes that will require the add-on to be significantly rewritten. If you think that's the case, it's OK to resolve the review without proceeding to the next step. If the add-on needs work but is safe to use, it can be given a preliminary review approval without performing any testing. If the add-on needs work and you are not sure if it is safe to use, then leave the add-on for someone else to review, request a admin review, or ask for help in the mailing list.
=== Step 4: Feature Review ===
The last step in a review is to install and test the add-on.
==== Testing setup ====
These are a few settings and tools you should use or consider using when setting up your add-on testing environment:
*'''Always use a separate profile for testing''', never your main profile. See [https://developer.mozilla.org/en/Setting_up_extension_development_environment Setting up an extension development environment].
*Ideally you should perform your tests in a virtual machine. It is always useful in case you need to test in multiple platforms. [http://www.virtualbox.org/ VirtualBox] is free and supports most platforms.
*Enable full Error Console reporting, as described in [https://developer.mozilla.org/en/Setting_up_extension_development_environment#Development_preferences the Development Preferences section].
*These add-ons help you inspect add-on behavior and find possible solutions for problems:
**[https://addons.mozilla.org/en-US/firefox/addon/6622 DOM Inspector]. Analyze XUL and HTML layout, CSS and even JS objects.
**[https://addons.mozilla.org/en-US/firefox/addon/1815 Console<sup>2</sup>] for detailed Console logging.
**[https://addons.mozilla.org/en-US/firefox/addon/966 Tamper Data] or [https://addons.mozilla.org/en-US/firefox/addon/3829 Live HTTP Headers] for HTTP traffic analysis.
**[https://addons.mozilla.org/en-US/firefox/addon/2490 Leak Monitor] to detect some types of memory leak. See [https://wiki.mozilla.org/MozillaQualityAssurance:Home_Page:Firefox_3.0_TestPlan:Leaks:LeakTesting-How-To#Leak_Gauge Leak Gauge] for a more general solution.
**[https://addons.mozilla.org/en-US/firefox/addon/extension-test Extension Test] to detect loose variables and DOM IDs, prototype extension, dangerous category registration, and other difficult to spot problems. Also automatically sets the required dom.report_all_js_exceptions and javascript.options.showInConsole preferences.
*To test Fennec add-ons, you'll need to [https://wiki.mozilla.org/Fennec#Test_Builds install Fennec]. It is preferred that you test in a supported mobile device. If a Fennec nomination has been waiting for long, it's OK to test with the desktop XULRunner application.
*For online malware scanning, you can use [http://www.virustotal.com/ Virus Total], [http://www.kaspersky.com/scanforvirus Kaspersky online scan] and [http://virusscan.jotti.org Jotti online scan]. AMO performs virus checks, and binary add-ons should be admin-reviewed anyway, but if you suspect anything, those are good tools to use.
==== Installing and testing ====
Add-ons are normally cross-platform, so there will only be a single file to review, in the same section where the validator and source links are located. If the add-on is offered for a limited number of platforms, there will be individual links for each one of them. In this case all supported platforms should be tested.
Regarding applications, you don't need to test the add-on for all applications it supports. If the add-on supports Firefox and others, it's OK to only test on Firefox. If, however, an add-on update introduces Fennec or other application support, the add-on should be tested on it. The applications we support for reviews are listed on [https://addons.mozilla.org/en-US/firefox/pages/appversions this page].
==== Policies and actions ====
{| width="700" cellspacing="0" cellpadding="1" border="0"
|-
! style="border-bottom: 2px solid black" scope="col" | Policy
! style="border-bottom: 2px solid black" scope="col" | Action
! style="border-bottom: 2px solid black" scope="col" | Notes
|- style="vertical-align: top;" id="security-violations"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Security violations
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Adding HTTP content to secure pages. Visit HTTPS sites like addons.mozilla.org and make sure the identity button is unchanged. This is specially important for add-ons that insert scripts into sites.
[[Image:Identity-button.png|center|Identity button]]
|- style="vertical-align: top;" id="no-surprises-violations"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | [https://addons.mozilla.org/en-US/developers/docs/policies/reviews#section-defaults No Surprises] violation
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Changing homepage, default search provider, including unexpected ads or content changes without explicit user opt-in.
|- style="vertical-align: top;" id="privacy-violations"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Privacy violations
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Incorrect or insufficient privacy policies, not respecting Private Mode.
|- style="vertical-align: top;" id="modal-startup"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Showing a modal dialog at startup
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Many add-ons open dialogs or new tabs at startup, mostly offering information on getting started. This is useful, but it shouldn't block the user from using the browser. Opening modal (blocking) dialogs at startup is not allowed. Non-modal dialogs, separate windows or new tabs are allowed.
|- style="vertical-align: top;" id="errors"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Errors in the Error Console
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;" id="confusing"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Add-on is very hard to use without instructions
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | If the add-on is difficult to use, there should be instructions included in the add-on descriptions, or in a startup page or window.
|- style="vertical-align: top;" id="sticky-toolbar-buttons"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Toolbar buttons are not customizable
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;" id="leaks"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Memory leaks from content or chrome
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | If an add-on touches the content in any way, we need to test the add-on on a page where it works, close the page, and then look in about:memory?verbose. Remaining compartments related to the page indicate a memory leak. After disabling the add-on (and minimizing memory usage) the add-on should no longer use any memory. If it does, it leaks to chrome. More details [https://developer.mozilla.org/en/Extensions/Performance_best_practices_in_extensions#Avoid_Creating_Memory_Leaks here] and
[https://developer.mozilla.org/en/Zombie_Compartments#Proactive_checking_of_add-ons here].
|- style="vertical-align: top;" id="affiliate"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Affiliate linking
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | See details below.
|- style="vertical-align: top;" id="third-party-software"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Requires third party software or paid registration
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Admin Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | This excludes add-ons that require other add-ons to function, like Firebug extensions.
|- style="vertical-align: top;" id="content-ads"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Inserts ads into content
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Admin Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | The rules in these cases are complex. They need to be clearly labeled as coming from the add-on (otherwise Prelim). They can't remove or replace existing ads (otherwise Reject). They need to follow No Surprises (otherwise Prelim). And there are also security concerns (Reject) and privacy concerns (Prelim).
|}
Other tests to perform:
*Visit a very simple website like example.org and inspect its DOM, looking for any changes. Again, this is particularly important for extensions that insert scripts or make DOM changes.
*Open the add-on's preferences window, from the Add-ons Manager and elsewhere, and verify that preference changes apply properly. Make sure the window fits all of its contents (a common problem in Mac OS).
*Test all add-on features, within reason. If there too many, focus on the main features.
*Remove all added toolbar buttons, disable all added toolbars, and restart the browser. Make sure that buttons and toolbars are all removable and do not reappear on restart. Make sure that missing toolbar buttons to not cause errors to appear in the Error Console.
*Open the Customize Toolbar dialog and make sure that all buttons have appropriate icons and label text.
*Affiliate linking. Some add-ons add affiliate codes to Amazon links (or similar) in order to make money. At the moment we allow this as long as (1) the add-on follows the No Surprises policy, (2) the feature doesn't replace or remove any existing affiliate codes, (3) the affiliate codes aren't inserted in the merchant website's links (inserting Amazon affiliate codes in Amazon.com pages).
=== Step 5: Resolution ===
Choose the appropriate resolution and include all of your notes. Make sure you use a corteous and professional tone, and be as helpful as you can when pointing out problems or areas for improvement. If you are pushing the add-on public, thank the author for the time and effort they have put in. Once you're ready, click the ''Process Action'' button.
The submission of the review form can fail for a number of reasons, so it is highly recommended that you copy of your notes to the clipboard before you click on the submit button. You can also use add-ons like [https://addons.mozilla.org/en-US/firefox/addon/5761 Textarea Cache] for this.
'''Editor tour:''' remember to ask the admin editor to review your response before sending it.
[[AMO:Editors/EditorGuide/MailingList|Next: The Mailing List]]
'''Reviewer Intro Tour:''' ask your guide to select an add-on for you to review. Don't submit your first review without their pre-approval!
Add-on reviewers have a great responsibility. We need to ensure add-ons are safe to use, good quality, and clearly presented to our users. We also need to make sure developers get quick, clear, and actionable reviews.
AMO is designed so that popular and well-reviewed add-ons bubble to the top search rankings. The full and preliminary review levels also create two layers that allow us to list add-ons that are not quite ready for general consumption. Since we have good filtering mechanisms on AMO, our general policy is to '''only reject when strictly necessary'''. Rejection is necessary when an add-on has security issues, doesn't meet our content policies, or a few other cases which are spelled out in this guide. If an add-on doesn't meet the criteria for rejection, it should at least get preliminary approval.
=== Step 1: Review Add-on Metadata ===
Before getting started, here's a very important legal note: reviews '''must not''' involve checking for copyright or trademark violations. You should not take '''any''' action on an add-on because you suspect it copies code from others without permission or may otherwise infringe somebody else's copyright or trademark. The DMCA is a law that gives us legal protection from being held responsible for copyright infringement by users who post content to our site, but only if our conduct qualifies us for such protection and we follow exactly the procedures laid out in the DMCA. Determining copyright or trademark infringement is complicated and you will not have enough information to make those determinations.
If you have any concerns about the legality or legitimacy of an add-on, please email amo-editors AT mozilla DOT org.
==== Policies and Actions ====
{| cellspacing="0" cellpadding="1" border="0" style="width: 80%"
|+
|-
! style="border-bottom: 2px solid black" scope="col" | Policy
! style="border-bottom: 2px solid black" scope="col" | Action
! style="border-bottom: 2px solid black" scope="col" | Notes
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Empty name (the name is blank in default locale)
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Request More Information
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject if the name is not updated after 3 days.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Missing information in descriptions, missing testing information
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Request More Information
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Add-on name and code copied from very popular add-on (like Firebug or AdBlock Plus)
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Admin Review / Notify mailing list
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | These add-ons can include some form of malicious code and trick users into thinking they are the original one.
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Add-on name or icon identical or very similar to existing, active listing
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Admin Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | We want to avoid user confusion when selecting which add-on to install. It's OK to fork, but it should be clear what are the differences between very similar offerings.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Other copyright suspicions
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Ignore
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | We have a legal obligation *not* to take action unless the author of the original code files a DMCA complaint. If you see any malicious intent in the copied add-on, follow the same Action as the previous policy.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Not compatible with the current versions of the application
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Add note
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | There's a canned response for this purpose. Reject if the maxVersion is lower than 4.0, unless it's a critical fix.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Missing Privacy Policy or EULA when necessary
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | These descriptions are necessary if the add-on handles user information remotely, or the user needs to agree to any terms in order to use the add-on.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Questionable add-on relevance
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Carefully read the [[#Add-on_Relevance|Add-on Relevance]] section below. This policy should only be enforced on extreme circumstances.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | New version doesn't follow previous editor requests
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | If the author keeps submitting the same version, contact the mailing list.
|}
==== Add-on Relevance ====
Add-on relevance should only matter if the add-on is obscure or difficult to review, or you think that it is too simple to be fully approved. In general you shouldn't make any assessments on add-on relevance, since we have download counts, active daily users and user reviews to rank all add-ons on AMO.
We have a very low admission threshold, but we don't want to fully publish add-ons that will get at most a couple hundred downloads and will be mostly ignored during their lifetime. In that case, the add-on should at best receive a preliminary review approval.
Add-ons that commonly fall into this category are:
*Add-ons that help access very specific webpages. For example, an add-on that makes it easier to use an internal business site or a university library.
*Add-ons targeted to a limited geographical area. For example, an add-on for a local city newspaper or radio station.
*Add-ons that only add a toolbar or menu filled with website links, similar to plain bookmarks.
*Add-ons that download video files from Flash video sites like YouTube. We already have too many add-ons like these, with little to no distinction between them. Unless the add-on provides something really innovative to video downloading, it should not get full approval.
Assessing the relevance of an add-on can be difficult. In these cases you should leave the review to somebody else or ask on the mailing list.
=== Step 2: Automatic validation ===
We have a extensive set of static tests that identify common bad practices and possible security problems with add-on code. You must always run the code validator and inspect the results when performing a review.
Each Add-on History entry has a validation link. You'll want to validate the latest version.
[[Image:Validation-link.png|center|Add-on validation link]]
Clicking on the link will take you to the validation page, where the automatic code validator will run for that version of the add-on and then the results will be displayed. It's usually best for navigation to open this link in a new tab.
==== Policies and actions ====
{| width="700" cellspacing="0" cellpadding="1" border="0"
|-
! style="border-bottom: 2px solid black" scope="col" | Policy
! style="border-bottom: 2px solid black" scope="col" | Action
! style="border-bottom: 2px solid black" scope="col" | Notes
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Missing file / Parse error / Validation error
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Obfuscated, minified or binary code
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | There's a canned response for this.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Obfuscated, minified or binary code, with original sources included in XPI or provided link
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Admin Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Using eval, Function(), setTimeout, setInterval to evaluate remote code
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Remote script injection
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | <browser> or <iframe> elements with no type
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | See canned response and [https://developer.mozilla.org/en/XUL/iframe#a-browser.type iframe documentation]. If the frame or browser is used to load chrome content, it's fine as long as that's clear from reading the source.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Inserting remote content with innerHTML
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | If it's only local content, Add a Note to fix it on future versions. There's a canned response.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Using DOM Mutation events
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Just add a note if this is a first warning and the add-on has been approved with this code before.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Conduit add-on without [https://addons.mozilla.org/en-US/firefox/user/4959120 CONDUIT-AMO] as author
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Conduit add-on with [https://addons.mozilla.org/en-US/firefox/user/4959120 CONDUIT-AMO] as author
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Admin Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | JS Library (like jQuery) included, but not in its original file. JS Library doesn't pass checksum validation.
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | SHA-256 hashes recognized by the validator are stored at http://mzl.la/amo-libs and [https://github.com/mozilla/amo-validator/blob/master/validator/testcases/static_hashes.txt]
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Unicode characters (e.g. \u0060) in JS code
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Keep in mind these can be used safely inside strings. They're just not allowed to replace characters in JS code, since they can be used to bypass the validator.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Using eval, Function(), setTimeout, setInterval to evaluate local code
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | One case that we accept is when eval is used to replace existing Firefox functions. This is very common for add-ons that change bookmarking or tabbing behavior. It is also allowed in known libraries like jQuery.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Using the codebase_principal_support preference or enablePrivilege function
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | They're deprecated.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Native object prototype extension / Using Prototype library
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Storing passwords or other sensitive user data in the preferences
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Changing Firefox preferences without user consent
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | These include: network preferences, update system preferences, homepage, User Agent string.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Changing security preferences, permissions, certificates (nsIX509CertDB)
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Admin Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Using nsIProcess
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Admin Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Using JS c-types
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Admin Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Using Geolocation
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Test
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Give Preliminary Review if the add-on doesn't ask the user for permission to use geolocation before getting geolocation data ([https://developer.mozilla.org/en-US/docs/Using_geolocation#Prompting_for_permission this is how users should be asked for permission]). Approve if it does.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Localization errors
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Ignore
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|}
See the [[AMO:Editors/EditorGuide/AddonReviews/Security|security code review]] page for more detail on the above security rejection reasons.
Most of the other validator flags are not that important, but they should still be fully read and understood. When in doubt, check the help page or ask in the mailing list.
=== Step 3: Code Review ===
Every line of add-on code must be reviewed. The code validator can't detect all possible security or code quality issues, so we must always be on the lookout for bad code. For versions in the Preliminary Review queue, the code review should just ensure the add-on's safety.
All review pages have a View Contents link that take you to the code browser page. Pending updates also have a Compare with Public Version link next to it, which will show you the code with the changed sections highlighted.
For updates, the compare link should be used. However, it can sometimes fail to work for very large add-ons. In that case, you can either review the full source code, or download the new and public files to your system and compare them using tools like [http://code.google.com/p/amo-editor-tools/source/browse/trunk/sh/xpidiff.sh xpidiff.sh] or [https://wiki.mozilla.org/AMO:Editors/WinMerge WinMerge].
Validation results are integrated into the code viewer, so you can see validation warnings in context. For Add-ons SDK extensions, known library files are shaded in grey, and their origin is stated in the popup that appears when hovering over them.
==== Policies and Actions ====
{| width="700" cellspacing="0" cellpadding="1" border="0"
|-
! style="border-bottom: 2px solid black" scope="col" | Policy
! style="border-bottom: 2px solid black" scope="col" | Action
! style="border-bottom: 2px solid black" scope="col" | Notes
|- style="vertical-align: top;"
| 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;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Add-ons should not download remote code in any way. They are free to interact with web APIs as long as all that is being transmitted is data, not code. Add-ons are allowed to insert local scripts into webpages (within reason), but are not allowed to insert remote scripts.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Modified on unverified Add-ons SDK library code
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Library files that are unrecognized will not appear in grey color.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Using non-release Add-ons SDK library
| 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 or beta versions of the SDK. Others should only get preliminary review.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Using outdated Add-ons SDK library
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Except under special circumstances, 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="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | HTTP security violations
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| 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="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Bad or no namespacing
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | All scripts that are included in the main window overlay should have proper namespacing to avoid name conflicts with other add-ons. The name should normally correspond to the add-on name in order to guarantee its uniqueness.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preference names without "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. If the add-on is already public, this rule should be ignored.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Extension preferences names in Sync without "services.sync.extensions." prefix
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Add-on preferences using the Sync service of Firefox should use the "services.sync.extensions." prefix, and should also have a reasonable namespace.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Privacy issues
| 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.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Performance problems
| 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 requests, noticeably inefficient code, random UI freezes, loading large amounts of JS code (in the order of tens of thousands of lines of code) directly in overlays.
|- style="vertical-align: top;"
| style="padding: 0.5ex 1ex 1ex 0pt; border-bottom: 1px solid black;" | Not using prefwindow for preferences
| 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 window, we recommend that authors do 2 things: 1) use the ''prefwindow'' element, 2) add the line in install.rdf that enables this window to be opened from the Add-ons Manager window.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Duplicate / hidden files or folders
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Add Note
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | See canned response.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Unpacked add-on writes files in own installation directory
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Writing in the installation directory can generate problems with the Add-ons Manager, where the add-on can be stuck in an update loop. Add-ons should write all files in the profile dir.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Bootstrapped Add-on doesn't clean up after itself
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | See section on [[AMO:Editors/EditorGuide/SpecialAddonTypes#Bootstrapped_.28Restartless.29_Add-ons|Bootstrapped Add-on policies]].
|}
See the [[AMO:Editors/EditorGuide/AddonReviews/Security|security code review]] page for more detail on the above security rejection reasons.
In general you should apply your judgement and try to identify code that may appear suspicious or out of place. Try to understand what everything does and how it all fits together.
By the end of the code review, you'll may have a series of notes that will require the add-on to be significantly rewritten. If you think that's the case, it's OK to resolve the review without proceeding to the next step. If the add-on needs work but is safe to use, it can be given a preliminary review approval without performing any testing. If the add-on needs work and you are not sure if it is safe to use, then leave the add-on for someone else to review, request a admin review, or ask for help in the mailing list.
=== Step 4: Feature Review ===
The last step in a review is to install and test the add-on.
==== Testing setup ====
These are a few settings and tools you should use or consider using when setting up your add-on testing environment:
*'''Always use a separate profile for testing''', never your main profile. See [https://developer.mozilla.org/en/Setting_up_extension_development_environment Setting up an extension development environment].
*Ideally you should perform your tests in a virtual machine. It is always useful in case you need to test in multiple platforms. [http://www.virtualbox.org/ VirtualBox] is free and supports most platforms.
*Enable full Error Console reporting, as described in [https://developer.mozilla.org/en/Setting_up_extension_development_environment#Development_preferences the Development Preferences section].
*These add-ons help you inspect add-on behavior and find possible solutions for problems:
**[https://addons.mozilla.org/en-US/firefox/addon/6622 DOM Inspector]. Analyze XUL and HTML layout, CSS and even JS objects.
**[https://addons.mozilla.org/en-US/firefox/addon/1815 Console<sup>2</sup>] for detailed Console logging.
**[https://addons.mozilla.org/en-US/firefox/addon/966 Tamper Data] or [https://addons.mozilla.org/en-US/firefox/addon/3829 Live HTTP Headers] for HTTP traffic analysis.
**[https://addons.mozilla.org/en-US/firefox/addon/2490 Leak Monitor] to detect some types of memory leak. See [https://wiki.mozilla.org/MozillaQualityAssurance:Home_Page:Firefox_3.0_TestPlan:Leaks:LeakTesting-How-To#Leak_Gauge Leak Gauge] for a more general solution.
**[https://addons.mozilla.org/en-US/firefox/addon/extension-test Extension Test] to detect loose variables and DOM IDs, prototype extension, dangerous category registration, and other difficult to spot problems. Also automatically sets the required dom.report_all_js_exceptions and javascript.options.showInConsole preferences.
*To test Fennec add-ons, you'll need to [https://wiki.mozilla.org/Fennec#Test_Builds install Fennec]. It is preferred that you test in a supported mobile device. If a Fennec nomination has been waiting for long, it's OK to test with the desktop XULRunner application.
*For online malware scanning, you can use [http://www.virustotal.com/ Virus Total], [http://www.kaspersky.com/scanforvirus Kaspersky online scan] and [http://virusscan.jotti.org Jotti online scan]. AMO performs virus checks, and binary add-ons should be admin-reviewed anyway, but if you suspect anything, those are good tools to use.
==== Installing and testing ====
Add-ons are normally cross-platform, so there will only be a single file to review, in the same section where the validator and source links are located. If the add-on is offered for a limited number of platforms, there will be individual links for each one of them. In this case all supported platforms should be tested.
Regarding applications, you don't need to test the add-on for all applications it supports. If the add-on supports Firefox and others, it's OK to only test on Firefox. If, however, an add-on update introduces Fennec or other application support, the add-on should be tested on it. The applications we support for reviews are listed on [https://addons.mozilla.org/en-US/firefox/pages/appversions this page].
==== Policies and actions ====
{| width="700" cellspacing="0" cellpadding="1" border="0"
|-
! style="border-bottom: 2px solid black" scope="col" | Policy
! style="border-bottom: 2px solid black" scope="col" | Action
! style="border-bottom: 2px solid black" scope="col" | Notes
|- style="vertical-align: top;" id="security-violations"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Security violations
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Reject
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Adding HTTP content to secure pages. Visit HTTPS sites like addons.mozilla.org and make sure the identity button is unchanged. This is specially important for add-ons that insert scripts into sites.
[[Image:Identity-button.png|center|Identity button]]
|- style="vertical-align: top;" id="no-surprises-violations"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | [https://addons.mozilla.org/en-US/developers/docs/policies/reviews#section-defaults No Surprises] violation
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Changing homepage, default search provider, including unexpected ads or content changes without explicit user opt-in.
|- style="vertical-align: top;" id="privacy-violations"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Privacy violations
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Incorrect or insufficient privacy policies, not respecting Private Mode.
|- style="vertical-align: top;" id="modal-startup"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Showing a modal dialog at startup
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Many add-ons open dialogs or new tabs at startup, mostly offering information on getting started. This is useful, but it shouldn't block the user from using the browser. Opening modal (blocking) dialogs at startup is not allowed. Non-modal dialogs, separate windows or new tabs are allowed.
|- style="vertical-align: top;" id="errors"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Errors in the Error Console
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;" id="confusing"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Add-on is very hard to use without instructions
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | If the add-on is difficult to use, there should be instructions included in the add-on descriptions, or in a startup page or window.
|- style="vertical-align: top;" id="sticky-toolbar-buttons"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Toolbar buttons are not customizable
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" |
|- style="vertical-align: top;" id="leaks"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Memory leaks from content or chrome
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | If an add-on touches the content in any way, we need to test the add-on on a page where it works, close the page, and then look in about:memory?verbose. Remaining compartments related to the page indicate a memory leak. After disabling the add-on (and minimizing memory usage) the add-on should no longer use any memory. If it does, it leaks to chrome. More details [https://developer.mozilla.org/en/Extensions/Performance_best_practices_in_extensions#Avoid_Creating_Memory_Leaks here] and
[https://developer.mozilla.org/en/Zombie_Compartments#Proactive_checking_of_add-ons here].
|- style="vertical-align: top;" id="affiliate"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Affiliate linking
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Preliminary Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | See details below.
|- style="vertical-align: top;" id="third-party-software"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Requires third party software or paid registration
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Admin Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | This excludes add-ons that require other add-ons to function, like Firebug extensions.
|- style="vertical-align: top;" id="content-ads"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Inserts ads into content
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Admin Review
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | The rules in these cases are complex. They need to be clearly labeled as coming from the add-on (otherwise Prelim). They can't remove or replace existing ads (otherwise Reject). They need to follow No Surprises (otherwise Prelim). And there are also security concerns (Reject) and privacy concerns (Prelim).
|}
Other tests to perform:
*Visit a very simple website like example.org and inspect its DOM, looking for any changes. Again, this is particularly important for extensions that insert scripts or make DOM changes.
*Open the add-on's preferences window, from the Add-ons Manager and elsewhere, and verify that preference changes apply properly. Make sure the window fits all of its contents (a common problem in Mac OS).
*Test all add-on features, within reason. If there too many, focus on the main features.
*Remove all added toolbar buttons, disable all added toolbars, and restart the browser. Make sure that buttons and toolbars are all removable and do not reappear on restart. Make sure that missing toolbar buttons to not cause errors to appear in the Error Console.
*Open the Customize Toolbar dialog and make sure that all buttons have appropriate icons and label text.
*Affiliate linking. Some add-ons add affiliate codes to Amazon links (or similar) in order to make money. At the moment we allow this as long as (1) the add-on follows the No Surprises policy, (2) the feature doesn't replace or remove any existing affiliate codes, (3) the affiliate codes aren't inserted in the merchant website's links (inserting Amazon affiliate codes in Amazon.com pages).
=== Step 5: Resolution ===
Choose the appropriate resolution and include all of your notes. Make sure you use a corteous and professional tone, and be as helpful as you can when pointing out problems or areas for improvement. If you are pushing the add-on public, thank the author for the time and effort they have put in. Once you're ready, click the ''Process Action'' button.
The submission of the review form can fail for a number of reasons, so it is highly recommended that you copy of your notes to the clipboard before you click on the submit button. You can also use add-ons like [https://addons.mozilla.org/en-US/firefox/addon/5761 Textarea Cache] for this.
'''Editor tour:''' remember to ask the admin editor to review your response before sending it.
[[AMO:Editors/EditorGuide/MailingList|Next: The Mailing List]]
