Review Queue Progress and the v41 Release


(Alex Selkirk) #1

We submitted our add-on for preliminary review August 4th. It’s been 5+ weeks and we’re sitting at 69 in the queue. Given the changes in Firefox v41 releasing on (9/22), our users will be cut-off if our add-on is not reviewed by the add-on team very soon.

Obviously, we may also need to make changes to get approved in the review process. Looking at the past three weekly queue progress summaries, it appears that ~10 add-ons are being reviewed every week from the preliminary review queue; at that rate it seems extremely unlikely that we will be able to avoid orphaning our current users before 9/22. Based on the queue numbers, I don’t think we’re the only group in this situation. How are other folks preparing in terms of contingencies?

I know there are limited review resources, but is there an expectation that the rate at which add-ons are getting reviewed will increase in the next week? We’re just trying to figure out how to deal with this situation to minimize disruption for our users. Is there any chance of v41 release date moving back?

In addition, we have made significant updates to the client since the beginning of August, but have not updated the build in the queue for fear of losing our place in line. This is not an unusual situation for us, we frequently ship updates to our add-on monthly, and will not be able to maintain our Firefox extension support if we need to wait more than a month for those changes to reach our users. Sometimes when we are broken by unexpected changes, we need to ship updates in hours or cause significant disruption to our users. It would help a lot even if we knew that in order to get a build live by date x, we need to submit by date x-y (where y is days or ideally hours).

By contrast, our delay to ship our Chrome add-on is about 1 hour.

We want to support the Firefox platform, but are thinking now we may need to look to a more limited support model based on a bookmarklet to avoid the review queue.

Thanks,
Alex

ponder.co


(Elias Rabl) #2

I’m really concerned about this situation as well.

Having an unsigned add-on at the time Firefox v41 is going to be released will cause financial damage to the company i’m working for.
We would like to extend the functionality of our add-on until release date, but are afraid of loosing our place in the review queue. (Can somone confirm that this happens, if a new version is uploaded?)

And i’m pretty sure the release date won’t be postboned, so i’m considering to change the add-on visibility to unlisted: according to this site (Signing and Distributing your Add-on) unlisted add-ons are signed either immediately or reviewed within a couple of days (right now the waiting list = 18).
But the same article states:

Due to technical limitations on AMO, add-ons can only have their
visibility changed by deleting the add-on and submitting it again.

Edit: As i just relized this applies only when you would like to switch back from unlisted to listed. In this case you would have to change the UUID of the add-on and reupload it.

And you have to provide your own update channel for unlisted add-ons even though new versions of your unlisted add-on have to be signed via AMO again (actually it would be a great feature to update unlisted add-ons through AMO exactly like the listed ones…).

So this option might increase your chances of having a signed add-on when Firefox v41 is released but causes some proplems on the other side.

Greetings,
Elias


(Jerod Lycett) #3

A few things, first, back when you submitted in August you were likely at somewhere around position 400. Second, they appear to be focusing on full-reviews, so maybe ask for full not prelim. Third, these are volunteers for the most part, so they’re less likely to review if your code is cruddy as they actually have to read it. One of the best ways I’ve seen to get a quicker review is to ask politely.

As for unlisted, those all have to be admin reviewed as of right now (at least last I heard the auto-review wasn’t working).

I would suggest reading https://wiki.mozilla.org/AMO/Reviewers/Guide/Reviewing if you’re concerned about not making it.


(Andreas Wagner) #4

I’d say close to 100 is more accurate. However, add-ons are not strictly reviewed in the order of their waiting time and yours might be one of the more complicated ones or even flagged for admin review, which will take longer as we currently have very limited resources for admin flagged add-ons. I can’t say much more about your add-ons without knowing which one it is.

Mandatory signing has been delayed to Firefox 43.

I highly recommend uploading your newest version. You have nothing to lose. As I said, the queue position is not as important these days as it used to be.


(Andreas Wagner) #5

Please just resubmit your newest version. You will save reviewers the redundant review.

As I said, the signing has been delayed to Firefox 43. Switching to unlisted just for the sake of getting a faster review is does more harm than you gain. As you already said, how are you going to serve the update to your existing user base?

Please do not delete your add-on if you are going to resubmit it. Reviewers will lose all access to previous versions and reviews and will have to start reviewing all your code from scratch, which will significantly increase your waiting time.

No. If you want the amenities of AMO, then host your add-on on AMO.


(Andreas Wagner) #6

This is not correct. Add-ons are being reviewed on all queues. It’s rather a matter of complexity of the add-on review. Reviewers generally only review add-ons that are within their range of skills, which is a good thing. We onboarded several new reviewers recently, but we currently do not have a lot of senior reviewers and it just takes time to increase reviewing skills.

This is also not correct. We auto-approve hundreds of add-ons each week.


(Elias Rabl) #7

Thankks for the quick reply!

That’s some relieving news. This gives me enough time to submit our newest version, as you recommended.

Good to know - but kind of contradicting with my previous quote from this article - maybe you can update the note about “technical limitations on AMO […]”


(erosman) #8

@aselkirk & @elias_rabl
If you point out the addons you are talking about, we will see what can be done

When I was not a reviewer, I also had similar thoughts and the queue did no appear to be moving.

I am a volunteer reviewer. In the month of August, I personally reviewed 1286 addons and so far in September (13th) I have reviewed 624 addons.

It must admit that I usually only review addons with simple to medium complexity as the more complex ones are beyond my skill set.

It must also be said that some addons are needlessly complex.

I saw an addon with 529 JS/JSM files plus 523 JSON files.

I saw another one that started with 186 JS files and after rejection the next version had 2 JS files :open_mouth: (What were the rest for?)

I have seen addons that are 40-50mb … that is almost twice the size of Firefox.

Some Addons include complete Libraries (src/test/help etc) while they only need to use 1 of its files.
These addons will take longer to review.

Addons with Minified code, obscure Libraries, frameworks etc can take longer to review and often have to be reviewed by an admin reviewer.

I hope that helps


(Elias Rabl) #9

Hi ersoman,

i really appreciate the hard work you and the other reviewers are performing every day. It must be very exhausting to review so many different kind of addons, yet all of you are so nice and helpful. That’s what i love about the Mozilla community :slight_smile:

My addon (IDENTOS PW Manager) was actually reviewed and rejected from you in the past (i accidently packed a previous xpi into the new one - so you can count me to one of those goofs with strange review submissions…).

Since we are in the final testing phase it woul be great to show a signed version of our addon at business meetings. On the other side we might find some bugs/feature requests and have to submit a new version for reviewing.


(erosman) #10

@elias_rabl
I jsut reviewed your addon.

Please note:

Libraries
First consideration with Libraries is that they must be identical to the original version. It is not feasible to read and verify Library code as they contains 1000s of lines of code and are often minified. Therefore, they must be unaltered.
Furthermore, using lesser-known/unknown libraries complicates the matter more as they have to be verified first (by an admin reviewer).

innerHTML (or similar method of converting strings to DOM)
Use of innerHTML is generally not advisable.

Using innerHTML for any DOM that is NOT inserted into a document if acceptable. An example (that I have used in my own code) would be when addon grabs a selection from a page, creates a DIV, insert that selection into the DIV as innerHTML to check/verify/parse the content and the result is NOT inserted back into the document.

Using innerHTML to convert static (not dynamic) strings into DOM and insert into the DOM as a node, is allowed (but not recommended).
In case of dynamic values, there are more checks and if it is perceived to have security issues, it will be rejected.
Inserting remote content with innerHTML more than often results in rejection (and the sanitization are often not adequate).

Using innerHTML on a document node can be a cause for rejection as it will cause the document to be re-parsed, which is inefficient and has critical drawbacks, including invalidating any JavaScript reference to replaced DOM nodes, clearing any JavaScript properties and event listeners on replaced DOM nodes, and re-executing any script tags in the changed markup, and causing said scripts to fail.

I hope that helps


(Elias Rabl) #11

Thanks! I’m already working on a it.

The modified library jquery.popupoverlay.js is relativly easy to read with “only” close to 800 lines (10 of them added by me) - i’ll see if there’s a way to use the original lib. If i can’t find one, would it be possible to explain you the changes i made and still get it signed?

The innerHTML function is only called within jquery.dataTables.min.js which is the original library from https://www.datatables.net/ - modifying this file would result in the same problem as with the jQuery Popup Overlay library?


(erosman) #12

@elias_rabl
I did see the changes so no need to point out the changes, but that is a problem.

If it is a library that we accept, we dont check the code.
If we have to check the code, then it will takes longer. 800 lines of code have to read and checked line by line. That is often done by an admin which means we have to wait for an admin and it takes longer since there are fewer admins.

innerHTML or similar method …

These are some of the examples of converting strings to DOM

data/js/edit.js 5-14, 18-24, 68-71
data/js/inject.js 24-28
data/js/print.js 6-12
index.js 150-162


(Elias Rabl) #13

Thank you for approving my addon!

For inserting text, textContent (or JQuery text) or createTextNode() should be used instead of innerHTML.
For inserting HTML, the safer method is to use createElement(), textContent, appendChild() instead of innerHTML.

I’ll use the methods you mentioned in your email in the next version.

Is there a public list which libraries are accepted and which not?


(erosman) #14

There is this (old) list but it is not comprehensive:
https://github.com/mozilla/amo-validator/blob/master/validator/testcases/hashes.txt


(Alex Selkirk) #15

Thank you for the thorough answer!

Given all of this info, we’re going to submit our latest, but will go the unlisted route, as we handle distribution through our site registration process, and want to try to keep the update review process as streamlined as possible.


(Oscar Dedios Rodriguez) #16

It’s a pitty, but with the actual review process system is not worth to develop firefox extensions.

I appreciate the effort of the reviewers but there should be another system to protect the users from extensions with a permission request system like chrome or with I don’t know what, but wait some weeks for a revision of a couple of lines is crazy.

I have an extension that didn’t work anymore since one of the last new firefox versions. I changed a couple of things and I’m waiting the fu$%$ing review for 10 days, and I’m still in 115 position. In the mean while I’m loosing users and getting bad reviews.

I’m sorry but I’m not going to waste time in firefox anymore.

Oscar.


(Jerod Lycett) #17

The major issue is the developers have months of warning for changes, but don’t make the changes necessary until the last minute. When the original deadline for signing was early August most developers started uploading for review sometime in late July. They had known about it since late April. I’m assuming the version that you broke in is version 38, which moved through Nightly, Aurora (Dev Edition), and Beta before hitting Release. Each move was 6 weeks. You had 18 weeks (4.5 months) to make the changes.


(Oscar Dedios Rodriguez) #18

I didn’t have any warning neither any mail. I didn’t know that I should change anything until I realized the extension didn’t work anymore.
But, anyway, it’s possible It was my fault not to do test in nightly versions and visit regularly mozilla blogs.
But I don’t complain about the mozilla’s reviewers, they do great job. I complain about the system. It should change to some automatic system or develop to firefox is a nightmare.


(Wladimir Palant) #19

Unfortunately - no, there is no such system. Google is currently saying: “You installed a browser extension? Your problem!” Chrome’s permission system protects you against the extension taking over the browser, but that’s about it. Almost every extension requests full access to your browsing history because if it does something useful it can almost inevitably access your browsing habits. As a consequence, the warnings don’t mean anything, Chrome could just as well be asking: “Are you trusting this developer enough to install his/her extension?” The majority of Chrome extensions is spying on you, many will inject remote scripts into all webpages and could be altering your banking website without you noticing - there is nothing stopping Chrome extensions from doing that, not even Chrome Web Store policies.

Yes, the reviews are often annoying. Then again, I’m frequently rejecting extensions because of security issues - so maybe it has to be annoying so that people learn to build better extensions.


(Oscar Dedios Rodriguez) #20

I partially agree with you. Ok if the user accepts hosts permissions or browsing history it can be dangerous, Chrome doesn’t do any control about this. But a lot of people don’t trust in the extensions with this permissions. It’s more difficult to have installations with this permissions.

“The majority of Chrome extensions is spying on you” this is a fanboy sentence a great exaggeration, But is true that people should be careful with the extensions with hosts permissions. I have a popular extension in the Chrome Store and I’ve received a lot of mails offering me money for inject code with the host permissions (that my extension doesn’t have). (I don’t answer this emails).

Then I think a good idea it would be mix the better parts of each browser. Firefox would have a permissions rights that the users have to accept and if the extension has some riscky permission then a person has to review the code.

What do you think?

Regards.
Oscar.