Changes

Jump to: navigation, search

Add-ons/Reviewers/Guide/Reviewing

126 bytes added, 23:12, 22 January 2015
Proofreading
| 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;" | Other copyright suspicions .
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | No action
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | We have a legal obligation ''not'' to take action. See note at the beginning of this section.
|- style="vertical-align: top;"
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | Missing Privacy Policy 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;" | A Privacy Policy is required if an add-on sends any user information to a remote server, even if it is not personally-identifying. Even stats pings require a Privacy Policy.
== Add-on Relevance ==
Add-ons must not be rejected because a reviewer doesn't find them useful. We let AMO users make that call, and add-ons that aren't very useful won't gain much usage and have low search rankings, so they'll stay mostly out of the way.
In some cases, however, reviewers should deny full review to add-ons due to their usefulness. Add-ons that commonly fall into this category are:
| style="padding: .5ex 1ex 1ex 0; border-bottom: 1px solid black;" | eval can be allowed when it is used to patch Firefox functions with local code. setTimeout and setInterval can be used with hardcoded JS strings, but using closures is preferred.
|- 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;" | Add-ons can use data-only APIs, but should never download and execute remote code, not even in the scope of a webpage. Any use of the <script> tag (like createElement("script")) needs to be carefully analyzed.
|- 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 the [https://developer.mozilla.org/en/XUL/iframe#a-browser.type iframe documentation]. The type needs to be set to "content" (or "content-targetable", or "content-primary") ''before'' anything is loaded on that iframe. If the frame or browser is used to load chrome content, it's fine not to use a type as long as that's clear from reading the source code.
|- 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;" | The canned response points to the [https://developer.mozilla.org/en-US/Add-ons/Overlay_Extensions/XUL_School/DOM_Building_and_HTML_Insertion right documentation about this]. innerHTML will execute any JS code contained in the injected string, so it needs to be very clear that there is no executable code in it. The docs offer various ways to ensure this. Since this issue can often be confusing to developers, make sure you include a reference to a code file and line where this occurs.
|- 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. [https://developer.mozilla.org/en-US/docs/Web/API/MutationObserver Mutation Observers] are the recommended alternative.
|- 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;" | This only applies in XUL overlays, where the prototype extension affects the prototypes used by Firefox code and other overlays.
|- 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. They also must be restored to their previous values when the add-on is uninstalled.
|- 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;" | Request super-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;" | Request super-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;" | Request super-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;" | 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;" |
== 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 this code won't generate other validator messages, warnings and it is will be 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 framework 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.
| 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;" | 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="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;" | 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;" | 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." or "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 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="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 (Reject otherwise).
|- 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 (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;" | If the author keeps submitting the same version, contact the mailing list.
|- style="vertical-align: top;"
| 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="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.
Canmove, confirm
1,448
edits

Navigation menu