Langpacks don't handle versions properly

RESOLVED FIXED in Firefox 61

Status

P5
normal
RESOLVED FIXED
7 months ago
5 months ago

People

(Reporter: zombie, Assigned: gandalf)

Tracking

57 Branch
mozilla61
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox61 fixed)

Details

(Whiteboard: typescript-check)

Attachments

(1 attachment)

(Reporter)

Description

7 months ago
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`.
(Reporter)

Comment 1

7 months ago
Hi Zibi, can you please look into this?
Flags: needinfo?(gandalf)
(Assignee)

Comment 2

7 months ago
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)

Comment 3

7 months ago
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
(Assignee)

Updated

7 months ago
Flags: needinfo?(gandalf)
Comment hidden (mozreview-request)
(Assignee)

Updated

7 months ago
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
Flags: needinfo?(gandalf)

Comment 5

7 months ago
mozreview-review
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 hidden (mozreview-request)

Comment 7

7 months ago
mozreview-review
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+

Comment 8

7 months ago
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0cb6b7a530e2
Handle langpacks versions properly. r=aswan

Comment 9

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0cb6b7a530e2
Status: ASSIGNED → RESOLVED
Last Resolved: 7 months ago
status-firefox61: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61

Comment 10

6 months ago
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)

Comment 11

6 months ago
This bug was just removing some unreachable code, I don't think it needs manual testing.
Flags: needinfo?(gandalf) → qe-verify-
(Reporter)

Updated

6 months ago
Whiteboard: typescript-check

Updated

5 months ago
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.