245324 - Incompatible extension warning needs different text for app incompatibility than version incompatibility

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect, P4)

Description

[PikeUK]

When trying to install an extension targeted at another app (i.e. not Firefox)
the error message says "XXX will only work with Firefox (null)". It would be
better if it said something like "XXX is not compatible with Firefox". See the
attachment for an example.

Comment 1

[PikeUK]

Attachment: Test Extension targetted at a random GUID

Comment 2

[PikeUK]

Attachment: The warning dialog that appears with the previous XPI

Comment 3

[Nickolay_Ponomarev]

Pike, your test extension isn't downloadable. Could you attach it again?

The reason I'm asking is - with my dummy extension, targeted at a random GUID,
Firefox didn't show any alert at all. [20040606 aviary, installer].
(Details here: http://forums.mozillazine.org/viewtopic.php?p=565312#565312)

And if you have a later build, could you test if there is an error message, please?

Comment 4

[PikeUK]

Attachment: Test Extension targetted at a random GUID v2

The old one no longer installs on the latest builds (it was just an xpi
containing an install.rdf), this one has a jar file as well.

I've also uploaded it to my site in case you can't get it:

http://www.pikey.me.uk/mozilla/test/testext.xpi

I don't get any warning at all now either.

Comment 5

[PikeUK]

Uninstalling that last version seems to break the toolbars about 50% of the time
for me, can anyone else reproduce that?

Comment 6

[Nickolay_Ponomarev]

re: comment 5
bug 245553, now fixed, I guess. Try with 20040606 or later. If the problem _is_
with 20040606 or later, that bug should be reopened, I guess. Comment there, please.

see bug 245734 on the issue from comment 3

Comment 7

[PikeUK]

Attachment: Test Extension targetted at a random GUID v2 (the right one this time)

Aaaarggh, sorry for the spam I'm generating but the v2 extension I attached is
completely the wrong one, this is the right one. This bug is currently not
visible though due to Bug 245734.

Comment 8

[PikeUK]

Attachment: Test Extension targetted at a random GUID v3 (No comment)

Sorry. It takes real skill to screw up a simple testcase this many times, 4th
time lucky (hopefully).

Comment 9

[Benjamin Smedberg]

affects l10n, PR or bust

Comment 10

[:Mook]

Attachment: Possible fix

If extension has no minVersion for the right app, just say it is incompatible
(don't mention versions)

Testing against minVersion only in case bug 251148 is ever accepted.

Comment 11

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 156751 [details] [diff] [review]
Possible fix

>+  if (null == metadata.minAppVersion) {

rewrite this as "if (metadata.minAppVersion === undefined)" (test the code when
you do that to make sure it works - it should) ... the value strictly speaking
isn't |null| but |undefined| since the metadata obj was declared with var
metadata = { }; Please also add a comment here that reads "getItemMetadata does
not fill target application version range properties unless a matching
supported target application is found." since "metadata.minAppVersion ===
undefined" does not logically tell the reader of the code that there is an
incompatible app ID. 

Also, use -p when you make patches so I can find the code you're inserting into
easily. ;-)

>+  	params = [metadata.name, metadata.version, brandShortName];
>+    message = extensionStrings.formatStringFromName("incompatibleMessageNoApp", 
>+                                                    params, params.length);
>+  } else if (metadata.minAppVersion == metadata.maxAppVersion) {

Carriage return:

>+  } 
>+  else if (metadata.minAppVersion == metadata.maxAppVersion) {

.. in my files ;-) 

Make me a new tested patch I can apply and i'll check it in for you. Thanks :-)

Comment 12

[:Mook]

Attachment: addresses Ben's comments

Ben, thanks for the review!

The comment has been added (copy/paste).
Hopefully the CVS diff command used is better.	(Run patch from /mozilla/)
The brace style has been edited.
Tested with the dummy extension - didn't seem to give out any JS warnings or
otherwise break anything.

Comment 13

[Asa Dotzler [:asa]]

not going to hold the release for this but if you get the proper review before
PR, please request approval to land on the aviary branch.

Comment 14

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 157918 [details] [diff] [review]
addresses Ben's comments

r+a=[e-mail]

Comment 15

[timeless]

strictly speaking undefined can be defined to have some other value, using it in
code is therefore somewhat hazardous.

Comment 16

[Tracy Walker [:tracy]]

verified with Firefox branch build 2004-09-09-08-0.9

Comment 17

[Jolene]

latest java 6.0.30 compatibility needed.  

I recently updated to Firefox 9.0.1 and the java console 6.0.30 in add ons is disabled with a message "incompatible".  This java app is needed to run some features in Ancestry.com and needs to have the compatibility issues resolved. I have been corresponding with Ancestry.com's tech support and they stated that it needs the latest version of java in order to work correctly.  Coincidentally, everything worked fine in my previous version of Firefox prior to this 9.0.1 update.

Comment 18

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to Jolene from comment #17)
> latest java 6.0.30 compatibility needed.  
> 
> I recently updated to Firefox 9.0.1 and the java console 6.0.30 in add ons
> is disabled with a message "incompatible".  This java app is needed to run
> some features in Ancestry.com and needs to have the compatibility issues
> resolved. I have been corresponding with Ancestry.com's tech support and
> they stated that it needs the latest version of java in order to work
> correctly.  Coincidentally, everything worked fine in my previous version of
> Firefox prior to this 9.0.1 update.

Could you file a new bug, please? 
(This bug has been closed for 8 years, it's typically not useful to comment in such bugs, since they are generally not relevant any more.)