AMO review process (run-around)

4 min read Original article ↗

The following is a chronological log of my experience
in attempting to get this add-on to be reviewed/hosted
on the official Mozilla Add-Ons directory ("AMO"):

https://addons.mozilla.org/en-US/firefox/addon/json-dataview/

July 19, 2014:

version 1.0 is submit for a full review

July 24, 2014:

Your add-on, JSON-DataView 1.0, has been reviewed by an editor
and did not meet the criteria for being hosted in our gallery.

Reviewer:
"Leszek Zyczkowski" <lzyczkowski@aviary.pl>

Comments:
Your version was rejected because of the following problems:

  1. Your add-on creates DOM nodes from HTML strings containing potentially unsanitized data,
    by assigning to innerHTML or through similar means.
    Aside from being inefficient, this is a major security risk.
    For more information, see https://developer.mozilla.org/en/XUL_School/DOM_Building_and_HTML_Insertion

Please fix them and submit again. Thank you.

This version of your add-on has been disabled.

summary:

  • The conclusions drawn by the reviewer were incorrect, but understandable.
    They passed the code through a code analysis/validation tool,
    and it identified the presence of a few Javascript methods within the code base
    that are considered unsafe.
  • I saw the report, found the offending lines of code
    (all of which were located in obscure corners of 3rd-party libraries
    and completely unreachable from my code),
    and removed (commented out) those chunks of code.. as needed.
  • This was the basis for the following commit:
    f6003c8dcee37d164ac12f23aff7b5ba2680073d
    which produced the next minor version: 1.01

July 24, 2014:

version 1.01 is submit for a full review

July 24, 2014:

Your add-on, JSON-DataView 1.01, has been reviewed by an editor
and did not meet the criteria for full review.
However, your add-on has been granted preliminary review
and is now available for download in our gallery at
https://addons.mozilla.org/addon/json-dataview/

Reviewer:
"Leszek Zyczkowski" <lzyczkowski@aviary.pl>

Comments:
Dear add-on developer,

It is unclear to us at this time if your add-on will be useful for a general audience
so that it warrants public listing. We're granting you preliminary review only,
but encourage you to promote your listing so that it garners more active users and user reviews.
You may re-nominate your add-on once there is more proof of its usefulness.

Thank you.

Your add-on will now appear in search results and categories with some limitations.

Aug 03, 2014:

  • saved commit 8767637cb5469271f0e147a76f94c6a2a256c31f
    in response to two issues raised by a user:
  • as the diff shows, a trivial amount of code was changed.
    the update pertained to the conditional logic used to determine when to take action.

version 1.02 is submit for preliminary review

Aug 05, 2014:

Your add-on, JSON-DataView 1.02, has been reviewed by an editor
and did not meet the criteria for being hosted in our gallery.

Reviewer:
"XrXr"

Comments:
Your version was rejected because of the following problems:

  1. The included version of JSON Tree Viewer and JS Beautifier
    don't match our checksums for known release versions.
    We require all add-ons to use unmodified release versions,
    obtained directly from the developer's website.
    Please indicate which version of JS Beautifier you are using in the next version.

  2. There are a lot of repeated code in the add-on.
    To hasten the review process and improve readability, please refactor the code.

Please fix them and submit again. Thank you.

This version of your add-on has been disabled.

comments:

  • version 1.0 used an unmodified release version of JS Beautifier,
    and was rejected (in part) because of "unsafe code" found within the library.

  • JSON Tree Viewer cannot be used without modification.

    • it isn't written as a library
    • it uses "innerHTML" for all of its DOM creation
    • it makes assumptions about the DOM;
      specifically, that <ul id="tree"></ul> exists at page load

    Never-the-less:

    • its core is extremely well coded
    • it uses two functions to perform all of its DOM creation,
      so replacing "innerHTML" with W3C DOM API functions was pretty painless
    • it has a GPLv3 license,
      so it's all perfectly valid that I rewrote portions of its code.
      any changes I made were to replace hard-coded assumptions/dependencies
      with methods that allow the information to be passed as configuration options.
      the end result is a stand-alone library, which is what is required.

  • To be completely honest, I'm reaching the "use it / don't use it.. I don't really care" point.
    But, I'll give AMO another try..

  • Mainly because I do agree that the "nsIStreamConverter" components could probably be refactored.
    Each of these files corresponds to one of the supported 'content-types'.
    Each file is pretty much a copy/paste doppleganger, with the exception of 2 data values that vary.
    It wouldn't hurt to refactor them to all be generated by a common factory.

Aug 05, 2014:

version 1.03 is released (without submission to AMO)

Aug 06, 2014:

version 1.04 is submit for preliminary review