1185748 - Validation Results from addons.mozilla.org (Unknown file found in add-on, Obsolete element in install.rdf)

Status: RESOLVED FIXED

(Other Applications :: ChatZilla, defect)

Description

[scootergrisen]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:39.0) Gecko/20100101 Firefox/39.0
Build ID: 20150630154324

Steps to reproduce:

I tried uploading xpi language pack file to addons.mozilla.org.


Actual results:

I got these validation results:



Unknown file found in add-on

Warning: Files have been detected that are not allowed within this type of add-on. Remove the file or use an alternative, supported file format instead.

Detected file: install.js
install.js



Obsolete element in install.rdf

Warning: The element "requires" was found in the add-on's install.rdf file. It has not been banned, but it is no longer supported by any modern Mozilla product. Removing the element is recommended and will not break support.
install.rdf



Obsolete element in install.rdf

Warning: The element "file" was found in the add-on's install.rdf file. It has not been banned, but it is no longer supported by any modern Mozilla product. Removing the element is recommended and will not break support.
install.rdf


Expected results:

Fix it so there are no validation warnings or notices.

Comment 1

[scootergrisen]

Now i also get this notice:



Add-on contains JAR files, no <em:unpack>

Warning: The add-on contains JAR files and does not set <em:unpack> to 'true'. This can result in performance issues in add-ons that target Gecko 4 and above. You should either no longer use JAR files to package your chrome files (preferred) or add `<em:unpack>true</em:unpack>` to the install.rdf.
install.rdf

Comment 2

[:Gijs (he/him)]

Attachment: Patch

Fix all the things!

Comment 3

[:Gijs (he/him)]

Attachment: Patch

Attaching the right file would tend to help.

Comment 4

[James Ross]

Comment on attachment 8648031 [details] [diff] [review]
Patch

Review of attachment 8648031 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good overall; slightly concerned that there is nothing replacing <em:requires> but if there is something and we're already using it, or there actually is nothing (and the functionality is simply lost to time) that's okay.

::: locales/generic/install.rdf
@@ -20,5 @@
>  
>      <em:iconURL>chrome://chatzilla/skin/images/logo.png</em:iconURL>
>  
> -    <!-- Require ChatZilla. -->
> -    <em:requires>

So this is obsolete now, but what is its replacement?

::: xpi/makexpi.py
@@ +379,4 @@
>      progress_mkdir(jarroot)
>      print '                done'
>      
>      progress_echo('  Updating Firefox Extension files')

Nit: Drop "Firefox" from here like you did earlier in the file.

::: xul/content/static.js
@@ +523,5 @@
>          }
>      }
> +    else
> +    {
> +        client.host = ""; // We don't know this host. Show an error later.

This leaves client.unknownUID blank which might look a bit odd but I guess we're genuinely not expected to reach this code any more.

Comment 5

[:Gijs (he/him)]

(In reply to James Ross from comment #4)
> Comment on attachment 8648031 [details] [diff] [review]
> Patch
> 
> Review of attachment 8648031 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good overall; slightly concerned that there is nothing replacing
> <em:requires> but if there is something and we're already using it, or there
> actually is nothing (and the functionality is simply lost to time) that's
> okay.
> 
> ::: locales/generic/install.rdf
> @@ -20,5 @@
> >  
> >      <em:iconURL>chrome://chatzilla/skin/images/logo.png</em:iconURL>
> >  
> > -    <!-- Require ChatZilla. -->
> > -    <em:requires>
> 
> So this is obsolete now, but what is its replacement?

Nothing, afaict: https://bugzilla.mozilla.org/show_bug.cgi?id=647256, http://www.oxymoronical.com/blog/2010/01/Do-we-need-extension-dependencies .

> ::: xpi/makexpi.py
> @@ +379,4 @@
> >      progress_mkdir(jarroot)
> >      print '                done'
> >      
> >      progress_echo('  Updating Firefox Extension files')
> 
> Nit: Drop "Firefox" from here like you did earlier in the file.
> 
> ::: xul/content/static.js
> @@ +523,5 @@
> >          }
> >      }
> > +    else
> > +    {
> > +        client.host = ""; // We don't know this host. Show an error later.
> 
> This leaves client.unknownUID blank which might look a bit odd but I guess
> we're genuinely not expected to reach this code any more.

Yeah, I saw this and considering we don't support anything that doesn't implement appinfo, I figured it was basically dead code at this point.

Comment 6

[:Gijs (he/him)]

remote:   https://hg.mozilla.org/chatzilla/rev/dbeacae3a1f8