579170 - Reports of the default theme appearing to be incompatible

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Dave Townsend [:mossop]]

There are a number of reports coming in through Feedback of the default theme showing up as incompatible in the new add-ons manager.

I think we used to force a reload of install.rdf for all app-global extensions and I don't think we do anymore. That might cause this problem on some filesystems.

Comment 1

[Dave Townsend [:mossop]]

Actually that can't be the cause here since people are saying it has the right version number but wrong compatibility info. I think we might be overriding the new compatibility info on the new theme with the old info from the theme from the previous version.

Comment 2

[Dave Townsend [:mossop]]

Blair appears to be seeing this. The install.rdf for the theme in his install looks correct but the database does indeed say it is only compatible with 3.7a6pre which suggests I am probably right in comment 1.

Comment 3

[Robert Strong (they/them - no direct email)]

Keep in mind that some filesystems will update the directory's last modified time and others won't when the contents are modified in the directory.

Comment 4

[Dave Townsend [:mossop]]

(In reply to comment #3)
> Keep in mind that some filesystems will update the directory's last modified
> time and others won't when the contents are modified in the directory.

Yeah I think we should bring back forcibly refreshing the details for app-global anyway, but I'm pretty sure there is something more going on here.

Comment 5

[Robert Strong (they/them - no direct email)]

You might be able to check on an affected system by just modifying the install.rdf though I wouldn't be surprised if some editors / window managers on some platforms update the last modified time on their own. Should be easy enough to check if that is the case as well.

Comment 6

[Dave Townsend [:mossop]]

Attachment: patch rev 1

Filed bug to sort out forcibly updating metadata for app extensions on upgrade.

The problem here is basically what I thought. When we migrate data we do it by loading a small cache of the data about installed add-ons from the old DB including disabled state and compatibility information. Then as startup continues we effectively re-detect all add-ons as new and replace their data with that from the cache. This falls down if the add-on has been upgraded between the time that the old db was written and when we startup to create the new one, as happens with the default theme on app update.

The simple fix here is to also get the version of the add-on from the old database and compare it to the newly detected one. If they differ then we ignore the cached compatibility data, the other properties are still safe to apply though.

It is still possible to change the add-on without changing the version. The only way to handle that would be to add code to start loading extensions.cache during migration to get the old dir mtime, but that seems like too much work for what would be quite an edge case.

Comment 7

[Dave Townsend [:mossop]]

(In reply to comment #6)
> Filed bug to sort out forcibly updating metadata for app extensions on upgrade.

That should be "Filed bug 579513"...

Comment 8

[Robert Strong (they/them - no direct email)]

Comment on attachment 457969 [details] [diff] [review]
patch rev 1

Reasonable approach

Comment 9

[Dave Townsend [:mossop]]

Fixed: http://hg.mozilla.org/mozilla-central/rev/cc53b4e4e277

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Can't reproduce with builds like Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b4) Gecko/20100818 Firefox/4.0b4 ID:20100818121614. Given the fact the automated tests never failed, I will mark it as verified fixed.