918033 - Bootstrapped add-on update not updating string bundles cache

Status: RESOLVED WONTFIX

(Toolkit :: Add-ons Manager, defect)

Description

[Luís Miguel [:quicksaver] (not reading bugmail)]

Attachment: Manifest Test 1: One test string should be reported to the console

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20100101 Firefox/23.0 (Beta/Release)
Build ID: 20130814063812

Steps to reproduce:

1) Create a new profile
2) Install "manifest test 1" xpi
2.1) In the error console you'll see a message "First string test succeeded".
3) Install "manifest test 2" xpi;
3.1) You'll see the first message as before but it will fail to load the second message



Expected results:

This package contains a simple test.properties file, loaded via chrome.manifest during add-on startup. The startup() method in bootstrap.js uses Services.strings.* to call that .properties file and write in the error log its content.

"Test 2" contains an extra string in test.properties, which is also supposed to be written to the console, however it fails to do so when it is updated from "Test 1" and throws a warning to the console:

Warning: WARN addons.xpi: Exception running bootstrap method startup on manifesttest@quicksaver: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIStringBundle.GetStringFromName]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/XPIProvider.jsm -> jar:file:///C:/Users/Quicksaver/AppData/Roaming/Mozilla/Firefox/Profiles/c0wgkj18.Nightly/extensions/[e-mail]!/bootstrap.js :: startup :: line 7"  data: no]
Source file: resource://gre/modules/XPIProvider.jsm -> jar:file:///C:/Users/Quicksaver/AppData/Roaming/Mozilla/Firefox/Profiles/c0wgkj18.Nightly/extensions/[e-mail]!/bootstrap.js
Line: 7

It seems to be caching the files referenced in chrome.manifest when they are called in "Test 1"; then after updating to "Test 2" it seems to be using those cached files, which don't have the second string, instead of updating them to the files in "Test 2".

For a live example, go to https://addons.mozilla.org/en-US/firefox/addon/findbar-tweak/ and follow the same steps by installing the released version 1.3.4 and then updating it to the latest beta in the development channel. The add-on doesn't fail to load only because I have safeties in place to load from a backup "locale findbartweak-en[...]" if it can't find the strings it wants in "locale findbartweak[...]"; because that backup path wasn't called during execution of 1.3.4, it wasn't cached so it will be correct when called on 1.4b.

This isn't limited to "locale" declarations in chrome.manifest: on the FBT example, if you open the Find in All Tabs lists (Alt+F3), the filters button that is just above the splitter doesn't have an icon (which was not present in 1.3.4); because it's still using a cached version of chrome://findbartweak/skin/, it's not finding the icon. So it appears to that this also happens with "skin" declarations, and I have no reason to believe the same isn't also true for "content", except that I have done no tests on it.

Once you restart the browser, everything works as it is supposed to. I've reproduced this on Windows 7 and XP using Firefox 23, 24 and 25. I've not tested other OS/version combos.

Comment 1

[Luís Miguel [:quicksaver] (not reading bugmail)]

Attachment: Manifest Test 2: Two test strings should be reported to the console

Comment 2

[Luís Miguel [:quicksaver] (not reading bugmail)]

I'm sorry, I should have been more explicit about this. "Manifest Test 1" should report one string to the error console, "Manifest Test 2" should report two strings to the console.

Comment 3

[Luís Miguel [:quicksaver] (not reading bugmail)]

Does bug 830588 have anything to do with this, or is that related to content only?

Comment 4

[Luís Miguel [:quicksaver] (not reading bugmail)]

Should this really be specified to the Add-on SDK? None of my add-ons are built with it, nor are the examples I attached! They are straight up bootstrap.js + install.rdf + chrome.manifest files built by hand with no connection to the SDK itself.

Comment 5

[Dave Townsend [:mossop]]

We're not flushing the stringbundle cache when removing an add-on. We could dispatch "chrome-flush-caches" but I'm not sure if that might break other things.

You can probably work around this by appending a random ID to the end of the chrome url you pass to the string bundle service, like: chrome://manifesttest/locale/test.properties?32489765

Comment 6

[Luís Miguel [:quicksaver] (not reading bugmail)]

The problem with that is, I guess, every single bootstrapped add-on can suffer from this problem (in theory), so that would be a band-aid at best.

Also, it's not just stringbundles; "skin" declarations in chrome.manifest have this problem as well (chrome://someaddon/skin/*), and probably "content" as well; I can do some extra testing on this over the weekend if needed. I don't know if "chrome-flush-caches" would work for everything or just for bundles, so I wanted to make that clear in case it wasn't.

Comment 7

[Dave Townsend [:mossop]]

(In reply to Luís Miguel [:Quicksaver] from comment #6)
> The problem with that is, I guess, every single bootstrapped add-on can
> suffer from this problem (in theory), so that would be a band-aid at best.
> 
> Also, it's not just stringbundles; "skin" declarations in chrome.manifest
> have this problem as well (chrome://someaddon/skin/*), and probably
> "content" as well; I can do some extra testing on this over the weekend if
> needed. I don't know if "chrome-flush-caches" would work for everything or
> just for bundles, so I wanted to make that clear in case it wasn't.

content shouldn't have this problem, I think we already flush the correct caches for that, but by all means prove me wrong so we make sure we understand the full nature of the problem. "chrome-flush-skin-caches" should cover the skin problem.

Comment 8

[Luís Miguel [:quicksaver] (not reading bugmail)]

Attachment: Manifest Test - Baseline

Comment 9

[Luís Miguel [:quicksaver] (not reading bugmail)]

Attachment: Manifest Test - Content Locale Skin

Comment 10

[Luís Miguel [:quicksaver] (not reading bugmail)]

Attachment: FindBar Tweak Bug not reproducible in the Manifest Test

Comment 11

[Luís Miguel [:quicksaver] (not reading bugmail)]

So I went a little OCD and wrote a few tests for this.

Content Test: string value hardcoded in test.xul, value changed between versions
StringsBundle Test: string in test.properties, accessed from script tag in test.xul, string value changed between versions
DTD Entity Test: entity in test.dtd, inserted in test.xul, string value changed between versions
Image Test: image with src attribute hardcoded in test.xul, image changed between versions
CSS Test: image with list-style-image in css stylesheet, stylesheet points to different image in the second version
New Image Test: image in text.xul with src attribute, neither the image element or the image itself exist in the baseline

I also wrote a couple of tests for checking if the actual .manifest files are updated, but that seems to be working, so I'll skip those. I'll upload them if you want me to though.

To run the tests it's the same as before:
1) Clean profile.
2) Install "Manifest Test: Baseline"
3) Open "Manifest Test: Baseline" options dialog from the addons manager
4) Everything should read "Baseline"
5) Close the options dialog.
6) Install "Manifest Test: Content Locale Skin"
7) Open options dialog
8) Everything should read "Succeeded!"

As you expected, only stringsbundle seems to fail. This leaves me very confused, and a bit embarrassed admitedly. The reason I was so sure /skin/ was also affected is because of my example with FindBar Tweak (see screenshot):

1) Clean profile.
2) https://addons.mozilla.org/en-US/firefox/addon/findbar-tweak/
3) Install FindBar Tweak v1.3.4 (latest release)
4) Hit Alt+F3; the filters icon doesn't exist yet as expected
5) Install the latest v1.4b5 from the development channel
6) Hit Alt+F3; the place for the icon is there but there's no image!

At first, I found this when looking into the stringsbundle problem, so I presumed /skin/ was also affected. After seeing that's not the case, I inspected the element more closely (with DOM Inspector):
- The image element is there, visible, properly sized and positioned, clickable and fully functional
- The CSS code (list-style-image and -moz-image-region) is being properly applied to it
- If I open the icon URI in the location bar, it's opened without a problem

So... where did my icon go? Could this be a problem with the add-on itself (and if so, do you have any idea where I might want to start looking? Because I'm at a total loss...). Should I open a new bug about this? I still think it belongs in this bug, because after a restart everything is fine and the icon shows without a problem.

Also, your solution of adding a random number to the URI works, the icon appears with that; although this is weird because, like I said, opening the icon URI in the location bar (without the random number) opens the icon image properly.

Comment 12

[Luís Miguel [:quicksaver] (not reading bugmail)]

Correction:

New Image Test: image in text.xul with list-style-image in css stylesheet (scratch "with src attribute"), neither the image element, the css declaration or the image itself exist in the baseline

Comment 14

[Luís Miguel [:quicksaver] (not reading bugmail)]

(Quote Dave Townsend [:mossop] from bug 719376 comment #1, in case it might be still valid)
> Looks like just moving the notifications sent here
> http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/
> XPIProvider.jsm#1598 into flushStartupCache should do the trick, I don't
> remember if there are any adverse effects to doing those during runtime
> though.

Comment 15

[noitidart]

I think this might affect xbl files as well maybe? As i did some xbl restartless, and if i reinstall the addon after changing the xbl it doesnt seem to take the changes.

Comment 16

[Andrew Swan [:aswan]]

Extensions can no longer directly create string bundles.