If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Problem with automatic validation of a theme/extension

RESOLVED INVALID

Status

addons.mozilla.org Graveyard
Developer Pages
RESOLVED INVALID
4 years ago
2 years ago

People

(Reporter: Gerard Durand, Unassigned)

Tracking

Details

(Whiteboard: [contribute])

Attachments

(2 attachments)

(Reporter)

Description

4 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.

Wil?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(clouserw)
Nothing I know about.  Basta?
Flags: needinfo?(clouserw) → needinfo?(mattbasta)

Comment 3

4 years ago
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)

Comment 4

4 years ago
I can't reproduce the issue in validating it online:

https://addons.mozilla.org/en-US/developers/upload/5ed34061dab64c18b2247dac8e0028b0

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: https://addons-dev.allizom.org/en-US/developers/addon/noia-20-extreme-v3x/versions
(Reporter)

Comment 6

4 years ago
For your information, I have tried to upload exactly the file attached here, so a .xpi file.
Flags: needinfo?(Gerard-Florence.Durand)

Comment 7

4 years ago
I did some searches for where that error comes from, and it turns out that it's generated in Zamboni:

https://github.com/mozilla/zamboni/blob/master/apps/files/utils.py#L517

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:

https://github.com/mozilla/zamboni/commit/84128308aa0df5055625561be29c0a7e5716ad2c

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

https://github.com/mozilla/zamboni/commit/2f4c16d60d5c8d10b42628e1243f6d1b802d9e7d

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

https://github.com/mozilla/zamboni/commit/29aaca2ff1b12ebe5e54110e60e8892f2b5f1238#L6R75

It was created shortly before that by Jeff:

https://github.com/mozilla/zamboni/commit/dd82a21e4bc0256d05320ec0ee8b4b20f82e7047#L6R75

...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:
> 
> https://github.com/mozilla/zamboni/commit/
> 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.

Comment 9

4 years ago
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.
(Reporter)

Comment 11

4 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 ?

Comment 12

4 years ago
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.
 
https://github.com/mozilla/zamboni/blob/master/apps/files/utils.py#L81 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.
(Reporter)

Comment 14

3 years ago
Created attachment 8441461 [details]
V 3.48 version
(Reporter)

Comment 15

3 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.
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → INVALID
(Reporter)

Comment 17

3 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 ...
Thanks
(Assignee)

Updated

2 years ago
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.