Closed Bug 1455766 Opened Last year Closed Last year

Langpacks don't handle versions properly

Categories

(WebExtensions :: General, enhancement, P5)

57 Branch
enhancement

Tracking

(firefox61 fixed)

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: zombie, Assigned: zbraniecki)

Details

(Whiteboard: typescript-check)

Attachments

(1 file)

TypeScript check found that `.version` is not available here:

https://searchfox.org/mozilla-central/rev/14578d/toolkit/components/extensions/Extension.jsm#1826

and I didn't follow enough to see if the same is true for `.id`.
Hi Zibi, can you please look into this?
Flags: needinfo?(gandalf)
Hmm, I can see that we're not reading the version, but I'm not sure if we should do anything specific for Langpacks...

Every addon has a version, right? Andrew - can you advise on what I should do here?
Flags: needinfo?(gandalf) → needinfo?(aswan)
Yes, we use the version for langpacks eg here:
https://searchfox.org/mozilla-central/rev/14578d/toolkit/components/extensions/Extension.jsm#1811

Just grab that and store it in this.version I guess?
Flags: needinfo?(aswan)
Priority: -- → P5
Flags: needinfo?(gandalf)
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
Flags: needinfo?(gandalf)
Comment on attachment 8971387 [details]
Bug 1455766 - Handle langpacks versions properly.

https://reviewboard.mozilla.org/r/240146/#review246538

It looks like `Langpack.readLocaleFile()` is not reachable, so lets just remove it altogether?
Comment on attachment 8971387 [details]
Bug 1455766 - Handle langpacks versions properly.

https://reviewboard.mozilla.org/r/240146/#review246594

r=me with the commit message revised to describe the change
Attachment #8971387 - Flags: review?(aswan) → review+
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0cb6b7a530e2
Handle langpacks versions properly. r=aswan
https://hg.mozilla.org/mozilla-central/rev/0cb6b7a530e2
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
I have devised 2 tests for this:
1. I modify the id of the langpack and install it from file, this will result in corrupted file warning and the langpack is not installed, this behavior is not changed by this fix.
2. I have modified the version number of the langpack, this resulted in a caution unverified langpack for beta 60b13 and a corrupted file warning for beta 61b03.

Is this the desired behavior, for both version and id?
Flags: needinfo?(gandalf)
This bug was just removing some unreachable code, I don't think it needs manual testing.
Flags: needinfo?(gandalf) → qe-verify-
Whiteboard: typescript-check
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.