Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable all legacy add-on files #9472

Closed
jvillalobos opened this Issue Sep 20, 2018 · 15 comments

Comments

@jvillalobos
Copy link
Member

jvillalobos commented Sep 20, 2018

Following up on #9147 and the plan we published in the Add-ons Blog, we need to disable all legacy add-on files.

What needs to be disabled:

  • All legacy files of extensions and language packs, listed and unlisted, except for those internally signed by Mozilla.
  • All complete theme versions.

This should be done early in October to meet the timeline we laid out.

@grahamperrin

This comment has been minimized.

Copy link

grahamperrin commented Sep 23, 2018

… users will no longer be able to find …

Please: will the front end display anything distinctive in these cases?


Whilst I shouldn't expect to see the name of the non-found extension, it will be useful to use the word legacy somewhere in the page. Generic. Maybe with reference to https://blog.mozilla.org/addons/2018/08/21/timeline-for-disabling-legacy-firefox-add-ons/

TIA

@jvillalobos

This comment has been minimized.

Copy link
Member Author

jvillalobos commented Sep 24, 2018

Once the files are disabled, nothing about them will be visible on the site. Add-ons that have only legacy versions will also stop being visible or searchable.

@JustOff

This comment has been minimized.

Copy link

JustOff commented Sep 26, 2018

@jvillalobos, are you going to remove legacy dictionaries too?

@jvillalobos

This comment has been minimized.

Copy link
Member Author

jvillalobos commented Sep 26, 2018

No, legacy dictionaries were introduced in Firefox 61, which means they are still supported in the current ESR branch (60). We will probably remove them one the next ESR (68) becomes the norm.

@diox

This comment has been minimized.

Copy link
Member

diox commented Oct 9, 2018

QA: a command needs to be run to do this. Since it will disable a bunch of add-ons, let me know when you're ready for this to be done on dev & stage.

@AlexandraMoga

This comment has been minimized.

Copy link

AlexandraMoga commented Oct 9, 2018

@diox I think you can run the command for dev and stage and I'm going to make time to test around this. Do you also want to push the changes in prod this week?

@jvillalobos

This comment has been minimized.

Copy link
Member Author

jvillalobos commented Oct 9, 2018

We should wait for bug 1495440 to be resolved before we run this in prod. Other than being late on our timeline, there's no strong pressure to do this.

@diox

This comment has been minimized.

Copy link
Member

diox commented Oct 10, 2018

I've launched the script on dev - it worked except for a hundred or so files that failed because of a transaction issue. I'll be pushing a follow-up commit to work around this.

@diox

This comment has been minimized.

Copy link
Member

diox commented Oct 10, 2018

Looks good now. The query I used to verify:

File.objects.only('id').filter(is_webextension=False, is_mozilla_signed_extension=False, version__addon__type__in=(amo.ADDON_EXTENSION, amo.ADDON_THEME, amo.ADDON_LPAPP)).exclude(status=amo.STATUS_DISABLED).exclude(version__addon__status__in=(amo.STATUS_DELETED, amo.STATUS_DISABLED)).exclude(version__deleted=True).count()

The signal handlers in our code should have disabled the corresponding add-ons when necessary (when they didn't have any public versions left) and re-indexed them.

@diox diox modified the milestones: 2018.10.11, 2018.10.18 Oct 10, 2018

@AlexandraMoga

This comment has been minimized.

Copy link

AlexandraMoga commented Oct 12, 2018

Test results for AMO -dev (FF62, ESR60, Win10x64 and Android 8.0)

  • there are no visible regressions on the site
  • disabled legacy add-ons are not present on the homepage shelves, in collections, in user profile pages or on disco pane
  • only webextension versions are available to install on the add-on versions page
  • there are no complete themes available on the site
  • the language tools page on the frontend is listing only webext language packs; both legacy and webext dictionaries are listed

Dev Hub:

  • for developers that still had legacy add-ons (listed or unlisted) in their submissions list, these add-ons are going to appear as Incomplete and all version files are disabled; developers still have the possibility to upgrade their add-on to a webextension by submitting a new version

@diox I do have some questions:

Rev Tools:

  1. The Unlisted queue still includes disabled legacy add-ons. Should those records be removed?
  2. The Addon Review Log is holding entries for disabled legacy add-ons. Should they be cleared?

Admin

  1. I can still see disabled legacy add-ons, like Complete Themes or Legacy Extensions with "Approved" or "Awaiting Review" status, even though all their files have been disabled.
    Those listed as "Approved" in Admin are actually set as "Invisible" in Dev Hub, so far as I can see.
@diox

This comment has been minimized.

Copy link
Member

diox commented Oct 15, 2018

  1. and 2. : no. We only want to disable those add-ons & files for now, and that's it.
  2. I think this is because of 2 things: I forgot to include disabled add-ons when I ran the script ("invisible" add-ons are actually user-disabled) and in addition we have some stray add-ons in a weird state. I'll see what I can do to clean that up.
@diox

This comment has been minimized.

Copy link
Member

diox commented Oct 16, 2018

So the problem is that on dev, we have a bunch of (*) add-ons that have all their versions disabled but are still in a public state. This wasn't caused by my script (last modified dates for the versions I checked were in 2011), and because the versions are already disabled, the script is ignoring them.

I'll add a migration or something to clean them up later in #9707, once we've ran the command everywhere.

(*) 639 if my count is correct. They have no _current_version set but status is set to amo.STATUS_PUBLIC.

@diox

This comment has been minimized.

Copy link
Member

diox commented Oct 16, 2018

(Meanwhile: I ran the command on stage)

@vcarciu

This comment has been minimized.

Copy link

vcarciu commented Oct 16, 2018

I tested on Stage with same results as for -dev. Since the remaining issues described above will be addressed in a different bug, I will mark this one as verified.

@jvillalobos

This comment has been minimized.

Copy link
Member Author

jvillalobos commented Nov 12, 2018

Bug 1495440 has been resolved, so this is now free to be run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.