Closed Bug 1300735 Opened 3 years ago Closed 3 years ago
l10n data doesn't update on extension update, requires browser restart
58 bytes, text/x-review-board-request
Steps to reproduce: 1. Add a file locale/en-US.properties to your extension with the following line: foo=bar Also, put the following statement into lib/main.js: console.error("Value of foo: " + require("sdk/l10n").get("foo")); 2. Install extension. Press Ctrl/Cmd-Shift-J to open Browser Console - note the message "Value of foo: bar". 3. Now change the contents of locale/en-US.properties to the following: foo=not bar 4. Install extension. Press Ctrl/Cmd-Shift-J to open Browser Console. Expected results: The new message in Browser console says: "Value of foo: not bar" Actual results: The message says: "Value of foo: bar". Only after restarting the browser you get the correct value displayed. This is because of nsIStringBundleService caching string bundles. Add-on SDK needs to call nsIStringBundleService.flushBundles() when starting up, this will make sure that any strings are reloaded.
Comment on attachment 8788583 [details] [diff] [review] Proposed patch Review of attachment 8788583 [details] [diff] [review]: ----------------------------------------------------------------- This patch looks good, I'm not particularly used to this part of the SDK, but looking at how the `flushBundles` helper is used internally (currently it seems to be used only in the Fennec internals) the change makes sense. I was initially a bit unsure about the module choosen to apply the change, but after looking into the 'sdk/l10n' directory, I agree that the one choosen seems to be the better one. I'm going to set the f+ flag on this patch, and I'm converting the patch into an hg patch that can be applied unmodified to mozilla-central and I'm going to push it to try (given the change the jetpack testsuite should not behave differently from how it behaves on the current version of the SDK, but it is always better to check it on try before landing a patch, than discovering it from a backout message). I'm not a Peer of the SDK, and so I cannot set the final r+ on the this patch, but I'm going to set the r? flag assigned to Matteo (:zer0) on the patch converted to hg (and submitted to try) that I'm going to attach, then Matteo is going to completing the review on it.
Attachment #8788583 - Flags: review?(lgreco) → feedback+
Comment on attachment 8788583 [details] [diff] [review] Proposed patch Marking obsolete. This attachment has been converted to a hg patch (Attachment 8788935 [details]) which can be directly applied to a mozilla-central repo (with the author metadata preserved).
Attachment #8788583 - Attachment is obsolete: true
Comment on attachment 8788935 [details] Bug 1300735 - Flush cached locale strings when l10n SDK module is loaded. Re-new the f+ flag on the patch converted to hg.
Attachment #8788935 - Flags: feedback+
Try build, restricted to the mochitest-jetpack test suite, related to this change (attachment 8788935 [details]): https://treeherder.mozilla.org/#/jobs?repo=try&revision=0f8c3a9622e3
Comment on attachment 8788935 [details] Bug 1300735 - Flush cached locale strings when l10n SDK module is loaded. https://reviewboard.mozilla.org/r/77254/#review76230 Looks good to me!
Attachment #8788935 - Flags: review?(zer0) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/142fd07295b0 Flush cached locale strings when l10n SDK module is loaded. r=zer0
You need to log in before you can comment on or make changes to this bug.