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

VERIFIED FIXED in mozilla1.7.4

Status

()

Toolkit
Add-ons Manager
P4
trivial
VERIFIED FIXED
14 years ago
6 years ago

People

(Reporter: PikeUK, Assigned: Ben Goodger (use ben at mozilla dot org for email))

Tracking

unspecified
mozilla1.7.4
Points:
---
Bug Flags:
blocking-aviary1.0PR -
blocking-aviary1.0 -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [have patch] need review - ben)

Attachments

(3 attachments, 4 obsolete attachments)

3.49 KB, image/png
Details
1.24 KB, application/x-xpinstall
Details
3.08 KB, patch
Ben Goodger (use ben at mozilla dot org for email)
: review+
Ben Goodger (use ben at mozilla dot org for email)
: approval-aviary+
Details | Diff | Splinter Review
(Reporter)

Description

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

Comment 1

14 years ago
Created attachment 149825 [details]
Test Extension targetted at a random GUID
(Reporter)

Comment 2

14 years ago
Created attachment 149826 [details]
The warning dialog that appears with the previous XPI

Comment 3

14 years ago
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?
(Reporter)

Comment 4

14 years ago
Created attachment 150231 [details]
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.
Attachment #149825 - Attachment is obsolete: true
(Reporter)

Comment 5

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

Comment 6

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

Comment 7

14 years ago
Created attachment 150262 [details]
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.
Attachment #150231 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Flags: blocking1.0+
(Reporter)

Comment 8

14 years ago
Created attachment 150566 [details]
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).
Attachment #150262 - Attachment is obsolete: true
Priority: -- → P4
Target Milestone: --- → Firefox1.0beta

Comment 9

14 years ago
affects l10n, PR or bust
Flags: blocking-aviary1.0PR+
Flags: blocking-aviary1.0-
Flags: blocking-aviary1.0+

Comment 10

14 years ago
Created attachment 156751 [details] [diff] [review]
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.

Updated

14 years ago
Attachment #156751 - Flags: review?(bugs)

Updated

14 years ago
Whiteboard: [have patch]

Updated

14 years ago
Whiteboard: [have patch] → [have patch] need review - ben
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 :-)
Attachment #156751 - Flags: review?(bugs) → review-

Comment 12

14 years ago
Created attachment 157918 [details] [diff] [review]
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.

Updated

14 years ago
Attachment #156751 - Attachment is obsolete: true

Updated

14 years ago
Attachment #157918 - Flags: review?(bugs)

Comment 13

14 years ago
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.
Flags: blocking-aviary1.0PR+ → blocking-aviary1.0PR-
Comment on attachment 157918 [details] [diff] [review]
addresses Ben's comments

r+a=ben@mozilla.org
Attachment #157918 - Flags: review?(bugs)
Attachment #157918 - Flags: review+
Attachment #157918 - Flags: approval-aviary+
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED

Comment 15

14 years ago
strictly speaking undefined can be defined to have some other value, using it in
code is therefore somewhat hazardous.
verified with Firefox branch build 2004-09-09-08-0.9
Status: RESOLVED → VERIFIED
Product: Firefox → Toolkit

Comment 17

6 years ago
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.
(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.)
You need to log in before you can comment on or make changes to this bug.