705551 - em:unpack warning should recommend unpacking inner jars rather than adding em:unpack

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Add-on Validation, defect)

Description

[Kris Maglione [:kmag]]

Per bug 551714 comment 23, we should be recommending that add-on authors either unpack their inner jar OR add em:unpack="true", with the former being preferred. A link to a MDC page wouldn't go amiss either. I'll find or write one if necessary.

This recently came up on the editors list and seems to be causing a fair amount of confusion, and we've gotten a lot of questions in the past.

Comment 1

[Kris Maglione [:kmag]]

Sorry, it looks like the contrary information in the mailing list thread completely negated my ability to read the actual warning. *goes to make more coffee*

Comment 2

[KLB]

The current addon validator warning about em:unpack is a little confusing. Without careful reading, it almost seems like the warning is saying that both unpack=true should be used AND that packing chrome documents in a JAR file should not be done. Instead the intention is an either/or situation with not packing chrome documents in JAR files is preferred. 

I know the warning lead me to both add em:unpack and to stop using JAR files within my extension.

The current wording is:
-----
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 Firefox 4. It is recommended that you consider no longer using JAR files to package your chrome files.
----

Maybe the wording should be something more like:
-----
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 Firefox 4+. 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.
----

Comment 3

[Michael Wu [:mwu]]

Well, the message is still not entirely true. There is minimal performance impact when putting jars inside the xpi *if* the jars inside the xpi aren't compressed. (though the files in the inner jar may be compressed if desired. The warning should not be emitted if that is the case - it usually takes a bit more work to go out of your way to do this.

However, that should be rare. I agree - the current message does sound like you should stop using jars and add <em:unpack> to true, which is probably the worst thing you can do.

Comment 4

[Kris Maglione [:kmag]]

mwu: There was some further discussion about this in the mailing list thread that spawned this bug. Giorgio Maone and Nils Maier have said that in their testing an uncompressed inner jar has lead to a slight performance increase. I can think of a few reasons that might be the case, but I was wondering if there's been any serious testing on the matter. I know that a lot of research went into the omni jar shift, and it seems odd that if it does tend toward a performance increase the technique isn't used in omni.jar. I was hoping to track down you or tglek this week so I can put together a definitive MDC entry for this warning.

Comment 5

[Jorge Villalobos [:jorgev] (he/him)]

It is worth noting that most add-on developers create their JARs by zipping the files, using default options, and then just rename them. Assuming the JARs are compressed is the right way to go.

Comment 6

[Jorge Villalobos [:jorgev] (he/him)]

Reclassifying editor bugs and changing to a new whiteboard flag. Spam, spam, spam, spam...

Comment 7

[Matt Basta [:basta]]

What is the decision on this bug?

Comment 8

[Jorge Villalobos [:jorgev] (he/him)]

Changing the message to what comment #2 says is the way to go.

Comment 9

[Matt Basta [:basta]]

Updated:

https://github.com/mozilla/amo-validator/commit/be0f66f475028e0b66a2529b247aa87b49634a4e