1455766 - Langpacks don't handle versions properly

Status: RESOLVED FIXED

(WebExtensions :: General, enhancement, P5)

Description

[Tomislav Jovanovic :zombie]

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`.

Comment 1

[Tomislav Jovanovic :zombie]

Hi Zibi, can you please look into this?

Comment 2

[Zibi Braniecki [:zbraniecki][: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?

Comment 3

[Andrew Swan [: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?

Comment 4

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: Bug 1455766 - Handle langpacks versions properly.

Review commit: https://reviewboard.mozilla.org/r/240146/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/240146/

Comment 5

[Andrew Swan [:aswan]]

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 6

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8971387 [details]
Bug 1455766 - Handle langpacks versions properly.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/240146/diff/1-2/

Comment 7

[Andrew Swan [:aswan]]

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

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0cb6b7a530e2
Handle langpacks versions properly. r=aswan

Comment 9

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/0cb6b7a530e2

Comment 10

[marius.santa]

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?

Comment 11

[Andrew Swan [:aswan]]

This bug was just removing some unreachable code, I don't think it needs manual testing.