Problem with automatic validation of a theme/extension



5 years ago
3 years ago


(Reporter: gerard-florence.durand, Unassigned)



(Whiteboard: [contribute])


(2 attachments)



5 years ago
Created attachment 810649 [details]
Noia2B2_Full_V3.xpi, version 3.47

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20100101 Firefox/23.0 (Beta/Release)
Build ID: 20130814063812

Steps to reproduce:

When I send a new update (v3.47) of Noia2 Theme for thunderbird, the validation window report an em:type error does not correspond to the module, and refuse to pass to the next step (the bottom buttom "Add a Version" to procced is still grayed). When I look in the detailed messages, I don't see any errors, all is OK, except warnings because some old syntaxes have been kept for backward compatibility. This sort of problem is not new and have been already met in the past with some of my previous versions. May be the reason is that my xpi file is in fact a composed one : so em:type 32, containing the theme itself (em:type 4) an a companion extention (em:type 2). This is not new, has been already like that in Noia2 TB theme for many years and exists also in some other themes for Thunderbird and Firefox (for example Silvermel, Charamel, NoiaFox, Noia4 for Firefox ...)

Actual results:

Validation refused due to an incorrect em:type message. But no precison where is exactly the problem.

Expected results:

Accept the theme then continue the process to be able to be examined by a reviewer.
I thought that one could upload complete themes with type=32 in install.rdf. This could be an edge case because it's a Thunderbird add-on, but I'd like to see if something changed recently, because there's a version that was uploaded and accepted in January this year.

Ever confirmed: true
Flags: needinfo?(clouserw)
Nothing I know about.  Basta?
Flags: needinfo?(clouserw) → needinfo?(mattbasta)
In looking at the package, I don't see anything immediately obvious (the em:type values look correct). We've supported multi-item extensions since the beginning, but they're more infrequent so they're not as well-vetted. I'll investigate more today.
Flags: needinfo?(mattbasta)
I can't reproduce the issue in validating it online:

One reason you might have gotten a failure is if you uploaded your add-on but it had a JAR extension instead of XPI. The validator will check that the add-on's file extension matches the em:type in the manifest.

Can you provide reliable STR?
Flags: needinfo?(Gerard-Florence.Durand)
There are no errors if you upload it to the standalone validator, but you see the type error if you upload it as a new version of the listing. I was able to reproduce this on -dev, on this page:

Comment 6

5 years ago
For your information, I have tried to upload exactly the file attached here, so a .xpi file.
Flags: needinfo?(Gerard-Florence.Durand)
I did some searches for where that error comes from, and it turns out that it's generated in Zamboni:

I don't know why it's there (we already check this in the validator anyway). The last person to touch that code was Rob Hudson about a year ago:

...but he only moved the message around. The same was done in January of 2011 by Jeff Balogh:

Prior to that, Jeff Balogh had changed the message (it was originally "<em:type> does not match existing add-on") in November of 2010:

It was created shortly before that by Jeff:

...which predates the creation of the validator entirely.

I'm actually at a loss of what to do here aside from simply remove the check. That would be an AMO devhub bug, though, and I have literally no experience working with any of that code at all. I'll let Wil work out the next step.
Component: Add-on Validation → Developer Pages
(In reply to Matt Basta [:basta] from comment #7)
> The last person to touch that code was Rob Hudson about a year ago:
> 84128308aa0df5055625561be29c0a7e5716ad2c

The last version of this add-on was uploaded successfully on January 2013, also with type=32 in install.rdf and an internal XPI and JAR, so the cause of the problem is likely somewhere else.
It's possible that it's being triggered through some other means, but that's the symptom. Either way, it's not--as far as I can tell--a bug in the validator.
If it's just duplicating a check that is done in the validator already, yeah, let's pull it out.

Comment 11

5 years ago
Already one month with this problem and several users asking for an update. I have my own web site where the update is available, but users don't always think to look at it.
If it's not a bug in the validator, where is the bug ?
If all this involves is ripping out old code, then that sounds like a great bug for the contractors/contributors. There's probably a test associated with this, as well, so that can also go.
Whiteboard: [contribute]
I don't know this code, but I suspect it won't be enough to just delete that if clause. has the list of em:types we support, and 32 isn't in there. In fact, I suspect the right move would be to keep the code that double-checks the type, but add support for Multiple Item Package (type=32) in the Extractor class.

However, it's very likely that's not enough, we need proper support of Multiple Item Packages, and I am not sure we have that in zamboni at the moment.

Comment 14

5 years ago
Created attachment 8441461 [details]
V 3.48 version

Comment 15

5 years ago
Comment on attachment 8441461 [details]
V 3.48 version

Apparently, this bug is still alive : I can't upload this new v 3.48 version. Always the same problem.
It looks that on firefox web site version, there was also this problem but it has been solved.
Hi Gerard,

I'm sorry for the extreme delay here.

It looks like the problem is that your previous versions included `em:internalName` attributes in your top-level install.rdf file, which overrode the `em:type` attribute, as far as Zamboni was concerned. Your new versions have this line commented out, which means that the detected type defaults to extension rather than theme.
Last Resolved: 4 years ago
Resolution: --- → INVALID

Comment 17

4 years ago
OK. I have re-added this em:internalName tag. Now it passes the automatic tests though this tag causes an informative warning. But, it works ! So ...
Product: → Graveyard
You need to log in before you can comment on or make changes to this bug.