How difficult is the extensions' code-review?

137 views
Skip to first unread message

Alessandro Vesely

unread,
Sep 25, 2011, 7:29:07 AM9/25/11
to dev-apps-t...@lists.mozilla.org
Hi,

I asked some explanation to the upstream developer of an extension
that I had not been able to find any more, Virtual Identity. The
response given [1] blames Mozilla's code-review and coding-
requirements. His blog allows replies, so I wrote one. But then,
I'm not much into extension development, so if some of you gurus
feels like posting the right suggestion, perhaps we can regain that
developer and his extension.

TIA
Ale

[1] http://blog.absorb.it/2011/09/24/whats-wrong-with-httpaddons-mozilla-org

--

Jonathan Kamens

unread,
Sep 25, 2011, 10:21:20 AM9/25/11
to
I just posted this response on the blog entry:

I maintain the Send Later 3 Thunderbird add-on, which has been
downloaded over 64,000 times and has over 15,000 active users.

If you want to do fast pre-release versions of your software
and get many people to test them, then you should be using the
beta channel on addons.mozilla.org (AMO). See
https://addons.mozilla.org/en-US/developers/docs/policies/main
tenance, where the use of the beta channel is fully documented.

You can ask users who are willing to ride the bleeding edge to
use the beta channel by (a) documenting it in your user guide
and (b) mentioning it to people who email you about your
add-on. In particular, when someone reports a bug to you, you
can fix it in a new beta release and let them know that the
fix is available in that beta release for them to download, as
long as you also make sure to explain to them that once they
download a beta release, they will continue to get new beta
releases automatically as they come out. Over time you will
build up a reasonably sized beta-tester community who will
report bugs to you and ensure that the add-on is stable before
you push a release to everyone. For this to work properly, the
release that you submit for general consumption should be
identical, aside from trivial changes, to the most recent beta
release.

I don't know how it worked in the past, but I'm under the
impression that with the current AMO, if you submit a new
release while the prior one was awaiting review, you get to
keep your place in line.

The coding standards that the Mozilla community is enforcing
on add-ons are reasonable and necessary. If these standards
are not enforced, then serious security and/or incompatibility
issues can arise. I don't always agree with the standards -- I
myself had an argument with the AMO editors about a change
they demanded that I make to one of my add-ons that I felt was
both unnecessary and inappropriate -- but I understand that
there is a need for standards and that just because I don't
always agree with them doesn't mean that I should throw in the
towel.

These standards do take time to enforce, which is why there
can be a delay between when you submit a new version for
review and when the editors are able to review it. If that
delay bothers you, then frankly, rather than throwing a
petulant tantrum and withdrawing your add-on from AMO, you
could volunteer to be an editor and review other developers'
add-ons to shrink the delay for yours. As far as I personally
am concerned, the delay is just a fact of life to be coped
with.

There's nothing wrong with publishing your add-on both on your
own web site and on AMO. It doesn't have to be an either/or.

Note that once your add-on is fully reviewed, users can
download the newest version from either your web site or AMO,
even if that new version hasn't been fully reviewed; they just
need to know to visit the "all versions" page on AMO to find
it there. If the changes in your new version are important
enough, then people will go to the trouble to find them. If
not, then it's really no big deal for them to have to wait a
few weeks until the new version is fully reviewed.

It is always bad for the community to lose a useful add-on or
a contributing add-on developer. I'm sorry you find the AMO
process and standards burdensome, but I hope you'll reconsider
your decision.

rene

unread,
Sep 25, 2011, 3:02:25 PM9/25/11
to dev-apps-t...@lists.mozilla.org
(no problem to discuss this here too...)

Hi Jonathan,

thanks for your extensive comment - all these instant reactions let me
think about my decision... but...

The process with publishing beta/pre-releases seems interesting. This
is exactly what I did with the users currently reporting a problem. I
released a bugfixed version on my own site and told them that they are
able to test it - with the problem that they will get all the betas from
my site till there is any stable release again. But if I like to release
some fix on AMO, it has to fulfill the coding standards, and I won't
change the old version that much anymore.

On Sun, 25 Sep 2011 14:21:20 +0000 (UTC), j...@kamens.brookline.ma.us
wrote:


> The coding standards that the Mozilla community is enforcing
> on add-ons are reasonable and necessary. If these standards
> are not enforced, then serious security and/or incompatibility
> issues can arise. I don't always agree with the standards -- I
> myself had an argument with the AMO editors about a change
> they demanded that I make to one of my add-ons that I felt was
> both unnecessary and inappropriate -- but I understand that
> there is a need for standards and that just because I don't
> always agree with them doesn't mean that I should throw in the
> towel.

Full ack.

On Sun, 25 Sep 2011 14:21:20 +0000 (UTC), j...@kamens.brookline.ma.us
wrote:


> I'm sorry you find the AMO process and standards burdensome,
> but I hope you'll reconsider your decision.

But what should I do if all my versions currently not accepted because
of the improved coding standards?
Yes, I will change my coding and maybe publish any other version on AMO
again. But till than I won't leave some buggy outdated version at this
site, only because the coding standards had been different one year ago.
The released AMO-version was worse than the current one, because beside
the rightfully critisized coding-crap it also contained some already
fixed bugs. There was even after asking no chance for me to publish this
version, nothing happens if the code does not fit the required schema.
The only option for me was to remove it from AMO. And thats what I did.

Nice regards, Rene

Jonathan Kamens

unread,
Sep 26, 2011, 9:56:06 PM9/26/11
to
rene <mozilla...@absorb.it> writes:
>But what should I do if all my versions currently not accepted because
>of the improved coding standards?

Fix the code.

I haven't found any of the coding standards imposed by the
AMO editors to be particularly burdensome. Really, I'd wager
that you could have fixed most of the issues they complained
about in the time it took you to post your blog entry, read
all the comments on it, and post your message here.

If you don't know how to fix the problems that are blocking
you from publishing your add-on, there are many available
resources. The AMO editors will respond to emails (sometimes
slowly, and sometimes they'll refer you elsewhere, but they
*will* respond). There are newsgroups where people are happy
to answer questions. There are IRC channels where people are
happy to answer questions.

If, for whatever reason, you really can't fix the code
quickly, then update the description of your add-on on AMO to
let people know that it's not the most recent version and give
them a link to the site with the most recent version on it.

But I gather it's too late for that, because you've already
removed your add-on from AMO, so now you can't get it back
into the search index until the AMO editors review and approve
it. If that's the case, then I suppose your decision to remove
your add-on from AMO may have been a bit rash.

rene

unread,
Sep 27, 2011, 4:05:02 AM9/27/11
to dev-apps-t...@lists.mozilla.org
Hi,

On Tue, 27 Sep 2011 01:56:06 +0000 (UTC), j...@kamens.us wrote:
> rene <mozilla...@absorb.it> writes:
>>But what should I do if all my versions currently not accepted
>> because
>>of the improved coding standards?
>
> Fix the code.

:) thats what I've done. And maybe it's easier for you to do that, for
me the 'namespace issue' was nothing to fix in a second.
It's done now in the new cutting-edge release, but I won't backport
these changes to the stable version which is just maintained for
compatibility issues. The new cutting-edge version dropped backward
compatibility, because there had been too many workarounds to cover all
the differences from TB 1.5 or Seamonkey 1.1 up to there current
versions.
And this is what really bothered me: Now I can't fix any small issues
in this stable version for Thunderbird pre 3.3 or old Seamonkey
anymore... if those fixes do not fit the new coding-requirements.

How do Mozilla does it? If there are severe issues, they are fixed in
there old versions too. Bigger changes are only going into the newest
version, as it should be. Why can't I work the same way?

AMO changed the criteria for the addons, but if my addon does not fit
the criterias anymore, then it should have been removed by them. Or
better, they might have blocked me from declaring it compatible to newer
TB or SM versions, ok. But they left it as it was and blocked me from
updating/fixing it, and that's why I removed it from AMO.

Anyway,
> ... Really, I'd wager
> that you could have fixed most of the issues they complained
> about in the time it took you to post your blog entry, read
> all the comments on it, and post your message here.
Meens that you are that fast in programming or that slow in
reading/writing? :) I was asked to declare more precise why I removed my
addon from mozillas site, and thats what I did.

Regards, Rene

Jonathan Kamens

unread,
Sep 27, 2011, 9:19:22 PM9/27/11
to
Rene,

I think I understand something now about what you are worried
about that I didn't understand before.

I think you're saying that you have multiple versions of your
add-on for different versions of Thunderbird, rather than
having one version that covers all supported versions of
Thunderbird.

You do forward development on the current versions but you
don't do forward development on the old ones -- you just
occasionally fix minor issues in them.

When you did that at one point, AMO suddenly declared that
they were no longer compatible with the newest AMO coding
standards and refused to grant them fully reviewed status.

You don't want to go back into those old versions and make
the significant changes that would be required for them to be
fully reviewed.

If I've gotten all that right, then I can see why you are
concerned, but my answer hasn't changed...

The coding standards are there for a reason. They are
necessary both to ensure that add-ons are secure and to
ensure that they don't break other add-ons and core
Thunderbird functionality.

The fact that AMO is now enforcing higher coding standards
when they were before does, to be sure, place an increased
burden on developers to adhere to those standards. It is,
nevertheless, both a good and necessary idea.

You have lots of options. You can (a) update the old versions
of your add-on to be compatible with the current coding
standards; (b) stop supporting the old versions of
Thunderbird and Seamonkey; (c) make the current version of
your add-on compatible with the old versions of Thunderbird;
or (d) remove your add-on from AMO.

It would seem that you have chosen (d). I would not have made
the same choice, but it's your choice, not mine.

I wish you good luck with your add-on, regardless of which
choice you make. (And just to be clear, I mean that sincerely;
I'm not trying to be snarky.)

rene

unread,
Sep 28, 2011, 1:28:14 PM9/28/11
to dev-apps-t...@lists.mozilla.org
Hi,

On Wed, 28 Sep 2011 01:19:22 +0000 (UTC), j...@kamens.us wrote:
> I think I understand something now about what you are worried
> about that I didn't understand before.
sorry, probably wasn't described clear enough...

> You have lots of options. You can (a) update the old versions
> of your add-on to be compatible with the current coding
> standards; (b) stop supporting the old versions of
> Thunderbird and Seamonkey; (c) make the current version of
> your add-on compatible with the old versions of Thunderbird;
> or (d) remove your add-on from AMO.
>
> It would seem that you have chosen (d). I would not have made
> the same choice, but it's your choice, not mine.
right, the other ones had been no options for me - because of different
reasons. So somehow I did the right step :(

Regards, Rene

Kent James

unread,
Oct 2, 2011, 7:31:31 PM10/2/11
to
Rene,

I am an extension writer who followed the suggested path of also being
an addon editor. I see that I reviewed Virtual Identity in March of
2010, but have not reviewed it recently.

You say that "over the years there had been more and more restrictions
at addons.mozilla.org". The "restrictions" in question are 1) addons
cannot cause a security problem that could potentially allow malicious
code to execute on a user's computer, and 2) addons cannot be written in
such a way to unnecessarily create potential conflicts with other
addons. I think if you told your users that you do not care about these
issues, they would not be enthusiastic about downloading the addon from
your site directly! Yes the "standards change" as addon editors get
better at identifying potential problems, but the underlying principles
and intentions have not changed.

Looking at your blog entry, it seems to me that what you are saying is
that you release software with bugs that need immediate fixes, and the
1-2 week process to turnaround the review of the bugfix is not
acceptable to you. Plus you do not have the time to do changes to your
code suggested by the addon editors to correct potential security
errors, and to reduce the potential for your addon to conflict with
other addons. It's understandable that you do not have the time to
maintain your addon - but your blog entry certainly leaves the
impression that you are unhappy with the addon review process, and not
simply that you are unwilling or unable to do the changes that are
needed. (See the OP of this thread for how your blog is interpreted).

You need to understand that an addon has tremendous power to allow
insecure access to a user's machine, so installing an addon is a lot
like installing a new program in your machine. Users are probably not
aware of that power and security risk. Malicious addon writers do try to
install key loggers and other malware through the addon process. So code
review is an important security step.

You also need to know that, at present, there are a whole lot more
addons needing review than there are people to do the the work. Wait
times for new addon reviews are now many months, though updates are much
shorter. (The Thunderbird update queue usually has a wait of 5-8 days.)
As a result, when an editor sees an unsafe coding practice, the standard
response is to deny full review of the addon without thorough tracing of
code to make sure that an unsafe practice does, in fact, lead to a
security breach. The alternative with existing staffing would be to do
thorough code tracing, and allow review queues to stretch out even
longer, when they are already much too long. I don't think that is what
most addon authors would want. But unfortunately addon authors sometimes
get very quick canned responses to common issues, which can be really
annoying if you have been waiting for a review. At least for updates of
Thunderbird addons, which I mostly do, I try to be appreciative of the
author's work - but the author needs to respect my time to, and do the
changes that were suggested (usually with a grace period of one release).

I hope you reconsider your decision and repost your addon on the Mozilla
addons site

rkent

Philip Chee

unread,
Oct 2, 2011, 9:34:10 PM10/2/11
to
On Sun, 02 Oct 2011 16:31:31 -0700, Kent James wrote:

> Looking at your blog entry, it seems to me that what you are saying is
> that you release software with bugs that need immediate fixes, and the
> 1-2 week process to turnaround the review of the bugfix is not
> acceptable to you. Plus you do not have the time to do changes to your
> code suggested by the addon editors to correct potential security
> errors, and to reduce the potential for your addon to conflict with
> other addons. It's understandable that you do not have the time to
> maintain your addon - but your blog entry certainly leaves the
> impression that you are unhappy with the addon review process, and not
> simply that you are unwilling or unable to do the changes that are
> needed. (See the OP of this thread for how your blog is interpreted).

He is saying that he only has time to do the changes suggested by addon
editors on his trunk development line and his latest release. His
supported "branches" are feature frozen and he only does bugfix updates
for those branches. He doesn't have time to do stylistic rewrites on his
feature frozen branches. Perhaps you would like to volunteer to be a
release driver for his LTS branches?

Phil

--
Philip Chee <phi...@aleytys.pc.my>, <phili...@gmail.com>
http://flashblock.mozdev.org/ http://xsidebar.mozdev.org
Guard us from the she-wolf and the wolf, and guard us from the thief,
oh Night, and so be good for us to pass.

rene

unread,
Oct 3, 2011, 4:42:22 AM10/3/11
to dev-apps-t...@lists.mozilla.org
Hi,

On Sun, 02 Oct 2011 16:31:31 -0700, Kent James wrote:
> Looking at your blog entry, it seems to me that what you are saying
> is that you release software with bugs that need immediate fixes, and
> the 1-2 week process to turnaround the review of the bugfix is not
> acceptable to you.

not that I like it to be that way, but thats the reality. I am not able
to test this addon with every software selection outside, and therefore
the problems on some setups are often found just at the day the software
is released to the majority of users. And it's not the beta-phase which
is missing, it's just that there is a difference between the few
beta-testing users and the whole user-group.
The only way to prevent this might be a straighter following of the
Thunderbird/Seamonkey development (check every commit for relevance),
probably combined with cleaner thinking and pogramming, which is for
different reasons not possible for me. I try it as much as I can, but
the reality stays different - the bug-reports are coming just after I
release a version. Therefore I require some immediate update-channel,
one week to react to a minore but ugly problem is to slow for me (and
the code-change is mostly minimal anyway).

> You need to understand that an addon has tremendous power to allow
> insecure access to a user's machine, so installing an addon is a lot
> like installing a new program in your machine.

I am aware of this fact.

On Mon, 03 Oct 2011 09:34:10 +0800, Philip Chee wrote:
> On Sun, 02 Oct 2011 16:31:31 -0700, Kent James wrote:
>> Looking at your blog entry, it seems to me that what you are saying
>> is
>> that you release software with bugs that need immediate fixes, and
>> the
>> 1-2 week process to turnaround the review of the bugfix is not
>> acceptable to you.

>> ...


>
> He is saying that he only has time to do the changes suggested by
> addon
> editors on his trunk development line and his latest release. His
> supported "branches" are feature frozen and he only does bugfix
> updates
> for those branches. He doesn't have time to do stylistic rewrites on
> his
> feature frozen branches. Perhaps you would like to volunteer to be a
> release driver for his LTS branches?

and thats the second point. To release better software and prevent
those issues with different software setups, I removed
backward-compatibility (and the related code) from the recent branch.
And because it was stable as it was, I disagreed in the requirement to
change the code of this stable release.

But anyway, for me removing the addon from AMO was the only way, and it
seemed to be the right one. The extension is probably not good enough
for AMO, and it's better if it does not have the 'tested by
AMO'-impression. And I won't blame the reviewers for there work, I just
expect that if some version was accepted for some Thunderbird/Seamonkey
release, than it is not clever to change the requirements for that
version (and for those Thunderbird/Seamonkey releases) later. Old
Thunderbird/Seamonkey's code won't fulfill the requirements too.

Nice regards, Rene

Reply all
Reply to author
Forward
0 new messages