375023 - Add-on update is not offered when it should be

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Public Pages, defect)

Description

[Varun]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.3) Gecko/20070309 Firefox/2.0.0.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.3) Gecko/20070309 Firefox/2.0.0.3

ColorfulTabs v1.9 on old AMO has been updated to v2.0 on new AMO (live now). Fx fails to detect updates even after clearing the cache, restarting the browser etc.

Reproducible: Always

Steps to Reproduce:
1. Download Colorful Tabs v1.9  from https://addons.mozilla.org/en-US/firefox/addons/versions/1368
2. restart browser and check for updates to Colorful Tabs.
3. No updates are detected. (The extension is however updated to 2.0 recently on new AMo which is now Live) https://addons.mozilla.org/en-US/firefox/addon/1368
Actual Results:  
No updates detected.

Expected Results:  
The updated v2.0 of ColorfulTabs should be detected.

This must be a generic issue with all extensions.

Comment 1

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Cleaning the cache should fix it, but doesn't. Setting to highest priority.

Comment 2

[Michael Morgan [:morgamic]]

I've been looking at this since we received the comment in our blog.  It has to do with minversions.  In SQL, the min version comparison between 2.0.0.x and 2.0b1 is not doing what we expect.

I think the solution is to avoid the minversion comparison in the update sql and just let the client handle the vercmp.

If you want to argue about it, read bug 304857 first.

If we remove the comparison, three things happen:
* we pull updates regardless of app min version, which could deliver non-workable updates to 1.5 or older clients
* user is allowed to download the file, but gets an error when it tries to install

The other alternative is to enforce minVersions that are legal versions.  I'm not sure that having a lower bound to these being "2.0b1" is really helpful.  Especially for public add-ons which are for public consumption.

So, I've taken a while to look into this.  I found that:
* v2 had an identical problem, this is not a new bug
* that was why v2 did not allow wildcards or non-numerical characters (aside from ".") in minVersions -- they could only be official releases
* remora is missing some of that logic for filtering minVersions, which is a bug (which can be argued, but I'm not a fan of picking non-official minVersions)

Arguments for supporting just any old minVersion:
* we want to test non-release versions!

Well, you can do that anyway by disabling the compat check, so I'm not sure there's much of a case for allowing stuff like this as minVersions:
* 2.0.0.* (infinite range)
* 2.0b1 (non-official release)
* 2.0.0.4pre (non-official release)

Comment 3

[Michael Morgan [:morgamic]]

To reiterate, after looking at all factors, I would rather implement the logic in the developer control panel to control minVersion by regular expression (as was done in v2).  I don't see a better alternative unless you guys can think of one.

->Critical because this is only affecting people with minVersions that are not "full" versions.

Varun, the quick thing to do would be to resubmit with a minVersion of 2.0.  Is there a particular reason why the minVersion is 2.0b1 instead of 2.0?

Comment 4

[Varun]

>>> user is allowed to download the file, but gets an error when it tries to 
>>> install

I guess that's the best way to handle it. 

>>> Varun, the quick thing to do would be to resubmit with a minVersion of 2.0.  >>> Is there a particular reason why the minVersion is 2.0b1 instead of 2.0?

Michael, my install.rdf reads em:minVersion="2.0"
what's the correct string for making minVersion to 2.0. For now, i went to the developer's control panel and set the minVersion to 2.0 Will this do the trick?

Thanks for the help

Regards
Varun

Comment 5

[Justin Scott [:fligtar]]

(In reply to comment #3)
> To reiterate, after looking at all factors, I would rather implement the logic
> in the developer control panel to control minVersion by regular expression (as
> was done in v2).

http://lxr.mozilla.org/update1.0/source/developers/additem.php#331

Although the error message seems to indicate that this sort of thing was checked, target app versions were only checked to exist in the database, not for min version acceptability. This would be new to remora.

Comment 6

[Michael Morgan [:morgamic]]

Yes, it will take a few minutes for the cache to clear and then it should work.  Thanks for reporting the bug Varun, we will work on making things clearer.

Comment 7

[Shawn Wilsher :sdwilsh]

(In reply to comment #3)
> Varun, the quick thing to do would be to resubmit with a minVersion of 2.0.  Is
> there a particular reason why the minVersion is 2.0b1 instead of 2.0?
> 

I know with my extension, I say the min version is 2.0 in the install.rdf, but the site changes it to 2.0b1

Comment 8

[Justin Scott [:fligtar]]

(In reply to comment #7)
> I know with my extension, I say the min version is 2.0 in the install.rdf, but
> the site changes it to 2.0b1

This does seem to happen with minversions - filing a separate bug.

Comment 9

[Frank Lion]

Mike (morgamic] Just to confirm details of Email reply. Metal Lion-Vista v1.6.2 is already set as a minVersion 2.0 in install.rdf 

<Description about="urn:mozilla:install-manifest">
         <em:id>{1AF3FC34-0725-4485-A939-6B40EB7CA96A}</em:id>
        <em:version>1.6.2</em:version>

    <!-- Target Application this extension can install into,
        with minimum and maximum supported versions. -->
        <em:targetApplication>
            <Description>
                <em:id>{ec8030f7-c20a-464f-9b0e-13a3a9e97384}</em:id>
                <em:minVersion>2.0</em:minVersion>
                <em:maxVersion>2.0.0.*</em:maxVersion>
            </Description>
        </em:targetApplication>

    <!-- Front End MetaData -->
        <em:name>Metal Lion - Vista</em:name> 

Comment 12

[Michael Morgan [:morgamic]]

The minVersions were coming across as something different from what users expected.  I think this is mostly a result of bug 375099, which should be pushed shortly.

For any of you who (wrongfully) have a minVersion of 2.0b1 you can set your minVersion in the database to match your install.rdf value using the Developer Control Panel.

Click on your latest version to edit the minVersion in the Edit page.

Comment 15

[Anonymous bin Ich]

I don't know how it is the bug#377323 is the same as this one, but I am no developer either.
Anyway, I looked into the install.rdf and it says:
SNIP:
    <em:targetApplication>
      <Description>
        <em:id>{ec8030f7-c20a-464f-9b0e-13a3a9e97384}</em:id>
    <em:minVersion>2.0b1</em:minVersion>
    <em:maxVersion>2.0b2</em:maxVersion>
      </Description>
:/SNIP

So I am guessing it requires only the update of the version for now. Am I wrong?

Comment 17

[Justin Scott [:fligtar]]

morgamic, I'll take this unless you were planning on working on it? (it's ASSIGNED to you currently)

Comment 18

[Robert Kaiser]

Prohibiting unofficial releases for minVersion makes it impossible to test new extensions against e.g. a Beta that first supports some feature the extension builds upon (see dictionaries, for example) and later extend the version range the extension can be used with to the final release after testing it works there (or even farther out at a later time).
If we need to resubmit the extension with a higher minVersion and (a higher extension version), that means that every user of that extension will be forced to redownload it, even if there were no real changes. In case of a 2.5 MB German dictionary with a quite big user base (2.3 million currently), that can be a quite unneeded load on AMO (and the users). I took that info from extension ID 3077, if you want to check.

Comment 19

[Michael Morgan [:morgamic]]

I agree, but I don't think the minVersion is for testing at all.  It's for what the extension is meant to be compatible with.

Why does it matter so much whether or not it is compatible with 2.0b1 if you know it should work with 2.0 (which is your target release, therefore compatibility should be implied)?

I don't think having minVersions and maxVersions that are not releases is a good idea, especially with the ability of nightly testers to disable extension compat checks.

I do agree, though, that having the change the install manifest everytime you bump a version in it sucks.  On the other hand, if you're just testing it should go in the sandbox and ideally it wouldn't be delievered automagically to the general users who need something stable.

We need to figure out how to handle sandbox updating and whether or not using non-major release versions (like a,b,pre,etc.) is prudent for "public" add-ons.  Until then we should aim for parity w/ v2.

Fligtar -- you can take this one.  I was waiting to see what happened with bug 375099 but that has long since been fixed.

Frank's problem was w/ his GUID mismatch.

Anupam, you are right, you should be able to fix it by advertising 2.0 and 2.0.0.*.

Comment 20

[Michael Morgan [:morgamic]]

(In reply to comment #19)
> I agree, but I don't think the minVersion is for testing at all.  It's for what
> the extension is meant to be compatible with.
> 
> Why does it matter so much whether or not it is compatible with 2.0b1 if you
> know it should work with 2.0 (which is your target release, therefore
> compatibility should be implied)?

Here I mean to say that we are really tracking two different types of "compatibility" -- one is public and one is not.

So I don't mean to say that version information for non-releases is completely useless.  I mean that it is not useful to non-testers who are using major release versions.

Comment 21

[Michael Morgan [:morgamic]]

Alright, I'm coming around on this.  Kairo has a valid point and in v2, after going back and reviewing coments I made about v2, the version.id auto increment value was later used and the primary sort for producing the update results.  See bug 334747.

Because it's the right thing to do we're going to let these versions fall where they may and adjust update behavior to honor them.

Comment 22

[Michael Morgan [:morgamic]]

Taking this back... and adjusting the title.

Comment 23

[Michael Morgan [:morgamic]]

To clear things up:
1) minVersions of 2.0.0.* are pointless and will not be allowed
2) minVersions of 2.0b1 are okay
3) update script will be updated in the AM to work with minVersions of 2.0b1, etc.

This bug was a bit confusing because there were three separate issues going on:
1) dynamic version numbers being changed from what they were in dev cp (bug 375099)
2) install.rdf values that had unreasonable max or min versions (causing clients to error when trying to update as in bug 377983)
3) reintroduction of the sql sorting issue (bug 334747)

These are, for the most part, edge cases -- most mainstream extensions have minVersions of 1.x or 2.x and have maxVersions of 1.5.0.* or 2.0.0.* already.

I feel less confused now -- hopefully that helps everyone understand what was going on here.  :)  Thanks for all your input.

Comment 24

[Michael Morgan [:morgamic]]

Update script was updated -- this particular bug should be fixed (having a minVersion like "2.0b1" will not prevent you from getting an update).

Having a minVersion like "2.0.0.*" or a maxVersion of "2.0b2" will still cause some problems, though -- because clients will think they are incompatible -- see bug 377983 for more on that.

Comment 25

[Robert Kaiser]

Yes, I'm with you that minVersions of x.* are useless, as are maxVersions of unofficial releases (alphas/betas).
The other way round it actually is useful, as even if you cound the sandbox, then you probably set a beta as minVersion when uploading into the sandbox, and people test your extension with beta successfully, then final is released, the unchanged version of your extension is still working well with the final, it can go public from the sandbox with just adjusting the correct maxVersion through the dev cp, and people who already tested the extension are not forced to re-download but can continue using the same version. So the minVersion of an unofficial version is pretty valid, but it's probably not as a maxVersion.
If I read this correctly, this works that way now anyways. Thanks for resolving this.