2000797 - Remove logic that forces distribution language packs to be reinstalled when upgrading from Firefoxes older than 67

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, task, P3)

Description

[Gregory Pappas [:gregp]]

if (addon.type === "locale" && oldAppVersion && Services.vc.compare(oldAppVersion, "67") < 0) {
  /* Distribution language packs didn't get installed due to the signing
       issues so we need to force them to be reinstalled. */
  Services.prefs.clearUserPref(XPIExports.XPIInternal.PREF_BRANCH_INSTALLED_ADDON + id);
}

https://searchfox.org/firefox-main/rev/d88792ab9de45efd74909d34fb20c8e8405dbb70/toolkit/mozapps/extensions/internal/XPIInstall.sys.mjs#4209-4219

This was added in bug 1551455 to work around an issue where preloaded Firefox on Acer laptops lost their language packs. It references "the signing issues" which is surely bug 1548973 based on the timing.

This seems like effectively dead code that could be removed but I'm not 100% sure.

Comment 1

[Gregory Pappas [:gregp]]

https://firefox-source-docs.mozilla.org/update-infrastructure/index.html#watershed-updates says that Firefox 72 is a 'watershed release' on all platforms. I believe this means any clients older than 67 will first upgrade to that before reaching a new version with this logic removed.

But also, since the old root certificate expired in March, would these language packs fail to install anyway, making this migration logic moot?

Comment 2

[Mike Kaply [:mkaply]]

Yeah, this can definitely go away.

Comment 3

[Luca Greco [:rpl] [:luca] [:lgreco]]

Hi Gregory,
you are definitely right, that's logic for very old version of Firefox migrating to more recent ones and given how fare in the past is Firefox 67 we are definitely happy to accept a patch with a cleanup of this area.

Are you interested in putting up a patch for this cleanup?
(I'd say that I would be more than happy if you are planning to, I really enjoyed working with you on the InstallTrigger cleanup).

Comment 4

[Gregory Pappas [:gregp]]

Thank you Luca!

I think this task would be a good first bug so I will mark it as such

Comment 5

[Aloys]

Hi Gregory. If no one is handling this could you please assign it to me?
I'm kinda new to open source and I think this would be a great place to get familiar with the contributing workflows.

Comment 6

[Gregory Pappas [:gregp]]

Sure! See https://firefox-source-docs.mozilla.org/contributing/contribution_quickref.html for more info on getting started

Comment 7

[Aloys]

(In reply to Gregory Pappas [:gregp] from comment #6)

Sure! See https://firefox-source-docs.mozilla.org/contributing/contribution_quickref.html for more info on getting started

Thanks!

Comment 8

[Aloys]

Attachment: Bug 2000797 - Remove obsolete language pack migration logic. r=gregp,rpl

This change removes the logic that forces distribution language packs to
be reinstalled when upgrading from Firefoxes older than 67

Comment 9

[Aloys]

Hey Gregory cc Luca,
I submitted the patch -> https://phabricator.services.mozilla.com/D273963

Also wanted to ask, since the linter flags oldAppVersion as defined but never used, I added the _. Wasn't sure whether to remove the parameter entirely or leave it for future purposes. The only caller of installDistributionAddon is https://searchfox.org/firefox-main/source/toolkit/mozapps/extensions/internal/XPIProvider.sys.mjs#3252.

Please let me know what you think when you get a chance to check it out.

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

This good-first-bug hasn't had any activity for 2 months, it is automatically unassigned.
For more information, please visit BugBot documentation.

Comment 11

[konyhéa]

Hii, can I pick this up?

Comment 12

[Rob Wu [:robwu]]

Aloys, we accidentally forgot to review your patch. Sorry about that. Do you want us to assign the bug to you again and review the patch, or should we assign the bug to someone else?

Comment 13

[Aloys]

Hey Rob.

Sorry about that.

It's okay, totally understand.

Do you want us to assign the bug to you again and review the patch, or should we assign the bug to someone else?

Yes. Since the patch was already submitted, please do reassign and review it. Thanks

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/192d726d25c2
https://hg.mozilla.org/integration/autoland/rev/3d7b36659e66
Remove obsolete language pack migration logic. r=rpl

Comment 15

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/3d7b36659e66

Comment 16

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/3d7b36659e66