Closed
Bug 1280378
Opened 8 years ago
Closed 8 years ago
Write system addon to direct users to download a fresh copy of Firefox from www.mozilla.org
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
RESOLVED
FIXED
People
(Reporter: spohl, Assigned: spohl)
References
(Depends on 2 open bugs)
Details
(Whiteboard: [go-faster-system-addon])
Attachments
(6 files, 3 obsolete files)
70 bytes,
text/x-github-pull-request
|
benjamin
:
review+
|
Details | Review |
70 bytes,
text/x-github-pull-request
|
molly
:
review+
|
Details | Review |
70 bytes,
text/x-github-pull-request
|
Felipe
:
review+
|
Details | Review |
70 bytes,
text/x-github-pull-request
|
Felipe
:
review+
|
Details | Review |
62.78 KB,
text/plain
|
Details | |
9.76 KB,
text/plain
|
Details |
We want to deploy a system addon to users who are clearly out-of-date. These users are also known as "orphaned users". Currently, users are considered orphaned when they use a version of Firefox that is at least 3 versions behind the current version. We may choose to deploy the system addon to more users at a later point (for example to users who are 2 versions behind). We met in London and agreed that the system addon should display a yellow bar across the top of web content in the browser, notifying the user that Firefox is out-of-date and it should direct the user to https://www.mozilla.org/firefox to download a fresh copy. This yellow bar should not be dismissable.
Comment 1•8 years ago
|
||
Just a note that you should exclude supported ESR versions in this add-on. Would this add-on be deployed to users on the release channel only or also to pre-release channels? For example we have a few Nightly users that are using old nightly branches, maybe pointing those users to nightly.mozilla.org instead would make more sense to them?
Assignee | ||
Comment 2•8 years ago
|
||
System addons can be targeted to specific versions of Firefox. Excluding supported ESR versions seems like a reasonable thing to do. I should have clarified that https://www.mozilla.org/firefox will only be the URL for users on the release channel. We will actually be looking at the app.update.url.manual pref to detect the proper URL to use. The app.update.url.manual pref refers to a URL appropriate for each channel. Also, based on bug 1213348, we can only deploy system addons as far back as Firefox 44.
Assignee | ||
Updated•8 years ago
|
Component: Application Update → General
Product: Toolkit → Firefox
Assignee | ||
Comment 3•8 years ago
|
||
I'm going to add telemetry in a separate patch.
Attachment #8764322 -
Flags: review?(felipc)
Comment 4•8 years ago
|
||
(In reply to Stephen A Pohl [:spohl] from comment #0) > We want to deploy a system addon to users who are clearly out-of-date. These > users are also known as "orphaned users". Currently, users are considered > orphaned when they use a version of Firefox that is at least 3 versions > behind the current version. We may choose to deploy the system addon to more > users at a later point (for example to users who are 2 versions behind). Do we have any information on how these users are spread across Firefox locales?
Assignee | ||
Comment 5•8 years ago
|
||
Forgot to |hg add| the file with localizable strings.
Attachment #8764322 -
Attachment is obsolete: true
Attachment #8764322 -
Flags: review?(felipc)
Attachment #8764326 -
Flags: review?(felipc)
Comment 6•8 years ago
|
||
I recall that :bs said that this wouldn't live in-tree? In particular as it'd target users of old versions of firefox?
Assignee | ||
Comment 7•8 years ago
|
||
(In reply to Francesco Lodolo [:flod] from comment #4) > Do we have any information on how these users are spread across Firefox > locales? Not yet, but I'll try to find out.
Assignee | ||
Comment 8•8 years ago
|
||
(In reply to Axel Hecht [:Pike] from comment #6) > I recall that :bs said that this wouldn't live in-tree? In particular as > it'd target users of old versions of firefox? I based this on a system addon from the tree. I do see your point about moving this out of the tree. Could you let me know where we should commit this, and how we should build it? Would this make localization more difficult?
Flags: needinfo?(l10n)
Comment 9•8 years ago
|
||
Right now, in-tree might even be harder than out-of-tree. The only piece that currently updates out-of-string-freeze is loop, and they develop out-of-tree, and code dump into mozilla-something, l10n and code-changes. A github repo where pontoon can write to would be easy enough. This can be the same repo where the code lives. Or it can be a separate repo with just the strings, and you pull things together at build time. The latter is what loop does, https://github.com/mozilla/loop-client-l10n. For a pretty static piece of code like this, though, just one repo probably works better. Flagging bsmedberg to chime in on the actual shipping target here.
Flags: needinfo?(l10n) → needinfo?(benjamin)
Comment 10•8 years ago
|
||
Not sure what "shipping target" means. Right now it's Firefox 44-46, and we'll extend the upper version as new Firefoxes get released.
Flags: needinfo?(benjamin)
Assignee | ||
Comment 11•8 years ago
|
||
I've run an analysis on our latest 1% longitudinal dataset from 6/20/2016. Keeping in mind that: 1. We can only ship system addons to versions 44 and above, and 2. Only version 44 is currently considered "orphaned", we wanted to know what the percentage of orphaned users is on version 44 and what the breakdown of locales is to prioritize localization. Results: 22.87% of orphaned users are running Firefox 44 and could benefit from this system addon. Locales are broken down as follows (Firefox 44): "en-US" : 39.45% "de" : 10.78% "fr" : 7.42% "es-ES" : 6.57% "pt-BR" : 5.68% "ru" : 4.52% "pl" : 3.01% "it" : 2.82% "zh-CN" : 2.19% "en-GB" : 2.14% "es-MX" : 1.90% "ja" : 1.15% "id" : 1.14% "nl" : 1.13% "tr" : 1.03% All other locales represent < 1%.
Comment 12•8 years ago
|
||
To clarify: We could reach 4 times as many users if we didn't ship this as a system addon? Also, I guess all other locales are below 1% each, but accumulate to ~9% of the 44 population?
Assignee | ||
Comment 13•8 years ago
|
||
(In reply to Axel Hecht [:Pike] from comment #12) > To clarify: > > We could reach 4 times as many users if we didn't ship this as a system > addon? If you're referring to hotfix addons, yes, potentially. However, I've been getting the sense that we were shying away from hotfix addons as much as possible moving forward and we can't target specific versions of Firefox server-side. We would therefore have to add smarts to only target versions of Firefox that are considered orphaned (not straightforward) in the addon itself while this can be done in Balrog with system addons. > Also, I guess all other locales are below 1% each, but accumulate to ~9% of > the 44 population? Correct. If you'd like these broken down further let me know and I'd be happy to do so. Just keep in mind that these numbers stem from the 1% longitudinal dataset and are therefore not perfectly precise. They should rather be seen as good estimates.
Assignee | ||
Comment 14•8 years ago
|
||
I've submitted the code to my GitHub repo: https://github.com/sapohl/outofdate-notifications-system-addon Benjamin was then able to fork it to: https://github.com/mozilla/outofdate-notifications-system-addon What I'm not entirely clear about is how these addons should be built as standalone addons. Felipe, do we have a mechanism in place for this? At the moment, the structure is still the same as if this was built as part of the tree. It used to be built as part of ./mach build by adding the addon to browser/extensions/moz.build (see attachment 8764326 [details] [diff] [review]), but this is obviously not possible if it doesn't live in tree.
Flags: needinfo?(felipc)
Assignee | ||
Comment 15•8 years ago
|
||
Comment on attachment 8764326 [details] [diff] [review] System addon This will live in a GitHub repo instead of in-tree, so canceling review.
Attachment #8764326 -
Attachment is obsolete: true
Attachment #8764326 -
Flags: review?(felipc)
Comment 16•8 years ago
|
||
If the strings are ready for localization, we need to set it up in Pontoon and send out an email to localizers. Do we have any ideas about deadlines? E.g. when the first batch of localization will be used, time before the next release, etc.
Flags: needinfo?(m)
Assignee | ||
Comment 17•8 years ago
|
||
Yes, the strings are ready for localization. The deadline is basically asap once code is reviewed and remaining questions regarding building have been resolved.
Comment 18•8 years ago
|
||
Matjaz, this sounds like a great opportunity to run a few of our PM folks through the process of project setup. Stephen, both flod and matjaz are PTO this week, so sadly we won't be able to make much progress until next week.
Assignee | ||
Comment 19•8 years ago
|
||
(In reply to Stephen A Pohl [:spohl] from comment #14) > What I'm not entirely clear about is how these addons should be built as > standalone addons. Felipe, do we have a mechanism in place for this? At the > moment, the structure is still the same as if this was built as part of the > tree. It used to be built as part of ./mach build by adding the addon to > browser/extensions/moz.build (see attachment 8764326 [details] [diff] [review] > [review]), but this is obviously not possible if it doesn't live in tree. Felipe, here are the things I'm not clear about: 1. Once the in-tree system addon is built, we get an output folder called "outofdate_notifications@mozilla.org" which can presumably be zipped up and made into an .xpi. Should we just create this folder on GitHub so that creating an .xpi is as simple as zipping up this directory? 2. Building converts the install.rdf.in into install.rdf after replacing @FIREFOX_VERSION@ with the current version of Firefox on trunk. Should we just hardcode this to a version such as 99.0 since we will control via Balrog who to deploy this system addon to? 3. Localization: each locale seems to create a folder (such as en-US) with an outofdate-notifications.properties file, a locale-specific manifest file (en-US.manifest) and this manifest file is referenced in the chrome.manifest. Is it possible to automate the creation and addition of new locales such that this does not need to occur at build-time? If the answer to all three questions above is yes, we can avoid actually "building" the addon, since we would simply need to zip up the root directory and change the file extension to .xpi. I could go ahead and upload the following structure to GitHub instead of what we have now: -- outofdate-notifications@mozilla.org ---- bootstrap.js ---- chrome.manifest ---- en-US ------ locale -------- en-US ---------- outofdate-notifications.properties ---- en-US.manifest ---- install.rdf
Comment 20•8 years ago
|
||
Not that I'm Felipe, but let me scribble in my answers to your questions. (In reply to Stephen A Pohl [:spohl] from comment #19) > (In reply to Stephen A Pohl [:spohl] from comment #14) > > What I'm not entirely clear about is how these addons should be built as > > standalone addons. Felipe, do we have a mechanism in place for this? At the > > moment, the structure is still the same as if this was built as part of the > > tree. It used to be built as part of ./mach build by adding the addon to > > browser/extensions/moz.build (see attachment 8764326 [details] [diff] [review] > > [review]), but this is obviously not possible if it doesn't live in tree. > > Felipe, here are the things I'm not clear about: > 1. Once the in-tree system addon is built, we get an output folder called > "outofdate_notifications@mozilla.org" which can presumably be zipped up and > made into an .xpi. Should we just create this folder on GitHub so that > creating an .xpi is as simple as zipping up this directory? Yes. > 2. Building converts the install.rdf.in into install.rdf after replacing > @FIREFOX_VERSION@ with the current version of Firefox on trunk. Should we > just hardcode this to a version such as 99.0 since we will control via > Balrog who to deploy this system addon to? Yeah, something like that. I don't have a strong opinion on the actual value. Maybe check in with the toolchain owners for signing this if there are restrictions on their side. > 3. Localization: each locale seems to create a folder (such as en-US) with > an outofdate-notifications.properties file, a locale-specific manifest file > (en-US.manifest) and this manifest file is referenced in the > chrome.manifest. Is it possible to automate the creation and addition of new > locales such that this does not need to occur at build-time? We don't have anything automated for the manifest stuff. Someone will need to do that manually, and I think it'd be easier to just do that in the chrome.manifest, instead of doing that twice. There shouldn't be any technical requirement to factor those out. > If the answer to all three questions above is yes, we can avoid actually > "building" the addon, since we would simply need to zip up the root > directory and change the file extension to .xpi. I could go ahead and upload > the following structure to GitHub instead of what we have now: > > -- outofdate-notifications@mozilla.org > ---- bootstrap.js > ---- chrome.manifest > ---- en-US > ------ locale > -------- en-US > ---------- outofdate-notifications.properties > ---- en-US.manifest > ---- install.rdf Sounds good to me, short of merging en-US.manifest into chrome.manifest.
Assignee | ||
Comment 21•8 years ago
|
||
This is great info, thanks Axel! I went ahead and made those changes. A pull request[1] is pending for the mozilla repo. Would you mind having a quick look to confirm that things look correct? Especially my attempt at merging chrome.manifest and en-US.manifest. Thanks! [1] https://github.com/mozilla/outofdate-notifications-system-addon/pull/1
Flags: needinfo?(felipc) → needinfo?(l10n)
Assignee | ||
Comment 22•8 years ago
|
||
Assignee | ||
Updated•8 years ago
|
Attachment #8766334 -
Flags: review?(felipc)
Attachment #8766334 -
Flags: feedback?(l10n)
Comment 23•8 years ago
|
||
Comment on attachment 8766334 [details] [review] System addon lgtm.
Flags: needinfo?(l10n)
Attachment #8766334 -
Flags: feedback?(l10n) → feedback+
Comment 24•8 years ago
|
||
Comment on attachment 8766334 [details] [review] System addon Looks good, except for the part where you're storing gNotification for the unload method. That gNotification is a global on your code's scope, and not per window as should be in the unload function. To fix it, no need to store gNofitication. In the unloadFromWindow function, once you get the high-priority-global-notificationbox, you can call .getNotificationWithValue("outofdate-notifications") on it and you'll have the right object to remove. Also, I'd name it something more official sounding that "Out of date notifications". Perhaps at least include Mozilla or Firefox in the name. This add-on name will show up in about:support and it's probably good to have something easily googleable and that doesn't look like it's an unrequested add-on. Other notes from our vidyo chat: - Use maxVersion as 50.* - test the two scenarios: - localized FF version with a language that has corresponding strings, to make sure they are picked up correctly - localized FF version with a language that don't have strings, to make sure that it falls back to en-US (although we should never ship to these users since showing them the message in a language that they don't understand won't be helpful). That got me thinking if the code itself should do something to check if the language is supported and, if not, don't display anything.
Attachment #8766334 -
Flags: review?(felipc) → review+
Comment 25•8 years ago
|
||
Does the tool that will be used to localize this only support localizing the properties/dtd files in the repo, or could we also ask the volunteers to localize the name/description in the install.rdf file?
Flags: needinfo?(l10n)
Comment 26•8 years ago
|
||
We don't support localizing install.rdf in-place, sorry. You'd need to do the thing with the .properties lookup. Re missing strings, that won't work by just doing the string bundle logic. you could add the en-US one explicitly as /content/, and fall back at run time. OTH, with the amount of strings in this add-on, just waiting 'til all strings are translated before adding them to the chrome.manifest might just be good enough.
Flags: needinfo?(l10n)
Comment 27•8 years ago
|
||
There is no reason to localize the install.rdf for a system addon: it never appears in any user-visible location.
Assignee | ||
Comment 28•8 years ago
|
||
Attachment #8767227 -
Flags: review?(benjamin)
Assignee | ||
Comment 29•8 years ago
|
||
Mark, in terms of next steps and deployment plan, here is what I had in mind: 1. Address addon code review, land in mozilla GitHub repo. 2. Get telemetry reviewed & landed in mozilla GitHub repo. 3. Sign addon with just en-US locale for preliminary staging/testing. 4. Get addon fully localized (all locales in same addon). 5. Sign addon with all locales. 6. Stage & test 7. Deploy to FF44. Does this seem reasonable? Am I missing any steps?
Flags: needinfo?(standard8)
Comment 30•8 years ago
|
||
There are two requirements before people can start using Pontoon to localize this addon. 1. Add https://github.com/mozilla-pontoon as a collaborator to https://github.com/mozilla/outofdate-notifications-system-addon. 2. Locale paths need to be changed to match the following pattern: “locale_code/same/path/for/all/locales”. E.g. if you turn “en-US/locale/en-US“ into “locale/en-US“, we're fine.
Flags: needinfo?(m)
Assignee | ||
Comment 31•8 years ago
|
||
Submitted pull request. (In reply to Matjaz Horvat [:mathjazz] from comment #30) > There are two requirements before people can start using Pontoon to localize > this addon. > > 1. Add https://github.com/mozilla-pontoon as a collaborator to > https://github.com/mozilla/outofdate-notifications-system-addon. Benjamin, could you handle this? I don't seem to be able to add collaborators. > 2. Locale paths need to be changed to match the following pattern: > “locale_code/same/path/for/all/locales”. E.g. if you turn > “en-US/locale/en-US“ into “locale/en-US“, we're fine. I changed this to locales/en-US (note the 's' at the end of locales, which seems to be standard for in-tree system addons).
Attachment #8768155 -
Flags: review?(benjamin)
Updated•8 years ago
|
Attachment #8768155 -
Flags: review?(benjamin) → review?(mhowell)
Assignee | ||
Comment 32•8 years ago
|
||
n-i for the below
> (In reply to Matjaz Horvat [:mathjazz] from comment #30)
> > There are two requirements before people can start using Pontoon to localize
> > this addon.
> >
> > 1. Add https://github.com/mozilla-pontoon as a collaborator to
> > https://github.com/mozilla/outofdate-notifications-system-addon.
>
> Benjamin, could you handle this? I don't seem to be able to add
> collaborators.
Flags: needinfo?(benjamin)
Comment 33•8 years ago
|
||
(In reply to Stephen A Pohl [:spohl] from comment #29) > Mark, in terms of next steps and deployment plan, here is what I had in mind: > > 1. Address addon code review, land in mozilla GitHub repo. > 2. Get telemetry reviewed & landed in mozilla GitHub repo. > 3. Sign addon with just en-US locale for preliminary staging/testing. I think you can test by hosting an unsigned xpi and some magic file. Rob Helmer knows more here, but we should get this documented, so I'll try and get more on that later. Is this also going to include a UX review? or has there been one already? (I don't see anything documented). > 4. Get addon fully localized (all locales in same addon). If you're using Pontoon, this is unlikely to be 100% localised, but getting at least some of it localised is a good idea. From comment 30 it looks like you can use the same repository - Loop used a separate repository (with a helper script) and exported/imported strings as appropriate, but I think its less of an issue for this add-on as strings shouldn't be changing frequently. > 5. Sign addon with all locales. > 6. Stage & test Having a test plan for QA here would also be good, especially on the first time around. Talking about testing, how about adding a functional test or something? This would make it easier for people thinking about deploying it to check it still does the expected prompts for each version. Loop is just switching to selenium+geckodriver as the supported route, although that's only supported from FF 48, but it may be the simpler way to go and give us protection for future changes. Maybe also helping to minimise QA. There's also taskcluster support that can easily be worked in. > 7. Deploy to FF44. > > Does this seem reasonable? Am I missing any steps? I think you want to also consider: - Tagging the repository + bumping version number before a release. If we're just shipping an existing version to a new version of FF, then it wouldn't need to be changed. - Having minimal deployment documentation. - Currently, you'll also need to be notifying release-drivers & creating a bug with xpi for shipping the add-on. See https://docs.google.com/document/d/1x27I7hAmWDWiqk3o3YC3fklhE3N59bdgHCQHF5p_lkU/edit# for more information. We can probably easily automate some of this if we want by copying from Loop.
Flags: needinfo?(standard8)
Comment 34•8 years ago
|
||
pontoon-l10n-robots is now granted write access to https://github.com/mozilla/outofdate-notifications-system-addon/settings/collaboration
Flags: needinfo?(benjamin)
Updated•8 years ago
|
Attachment #8768155 -
Flags: review?(mhowell) → review+
Comment 35•8 years ago
|
||
Thanks Stephen and Benjamin! The project is now set up in Pontoon. It will remain hidden until flod announces it to localizers.
Assignee | ||
Comment 36•8 years ago
|
||
Blocking 1256738 since this system addon is motivated by our analysis of telemetry data (see comment 11).
Blocks: 1256738
Comment 37•8 years ago
|
||
Comment on attachment 8767227 [details] [review] Telemetry I had already merged this.
Attachment #8767227 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 38•8 years ago
|
||
Thanks, Mark! I had a few followup questions. (In reply to Mark Banner (:standard8) from comment #33) > (In reply to Stephen A Pohl [:spohl] from comment #29) > > Mark, in terms of next steps and deployment plan, here is what I had in mind: > > > > 1. Address addon code review, land in mozilla GitHub repo. > > 2. Get telemetry reviewed & landed in mozilla GitHub repo. > > 3. Sign addon with just en-US locale for preliminary staging/testing. > > I think you can test by hosting an unsigned xpi and some magic file. Rob > Helmer knows more here, but we should get this documented, so I'll try and > get more on that later. Please let me know if you'd like me to n-i Rob here or if you were just going to get back to me. It would be great to have the ability to test this before requesting signing. > Is this also going to include a UX review? or has there been one already? (I > don't see anything documented). Yes, I worked with Bram leading up to MozLondon and in-person in London itself. Bram is happy with the current implementation. There may be minor changes in the future, but we have the go-ahead to proceed here. > > 5. Sign addon with all locales. > > 6. Stage & test > > Having a test plan for QA here would also be good, especially on the first > time around. I'll be reaching out to Ryan for assistance from SoftVision. > Talking about testing, how about adding a functional test or something? This > would make it easier for people thinking about deploying it to check it > still does the expected prompts for each version. > > Loop is just switching to selenium+geckodriver as the supported route, > although that's only supported from FF 48, but it may be the simpler way to > go and give us protection for future changes. Maybe also helping to minimise > QA. There's also taskcluster support that can easily be worked in. Is there documentation/examples that you could point me to? Do Loop's tests live in its GitHub repo? I have 0 experience with selenium+geckodriver. > > 7. Deploy to FF44. > > > > Does this seem reasonable? Am I missing any steps? > > I think you want to also consider: > > - Tagging the repository + bumping version number before a release. If we're > just shipping an existing version to a new version of FF, then it wouldn't > need to be changed. I don't understand this. Which repository should I be tagging? The GitHub repop with the addon? How do I tag a repo? Are you referring to bumping the version number of the addon? I'm assuming the current version of 1.0 would be sufficient for the initial release to FF44 and we'd bump it if we were to make any changes to it? Or would we also have to bump it if we were to start shipping it to FF45? For clarity, we want to ship this addon to FF44 at first. > - Having minimal deployment documentation. Is there an example for this? Or a list of things to include? Where will the documentation live?
Flags: needinfo?(standard8)
Assignee | ||
Comment 39•8 years ago
|
||
(In reply to Stephen A Pohl [:spohl] from comment #38) > (In reply to Mark Banner (:standard8) from comment #33) > > (In reply to Stephen A Pohl [:spohl] from comment #29) > > > 5. Sign addon with all locales. > > > 6. Stage & test > > > > Having a test plan for QA here would also be good, especially on the first > > time around. > > I'll be reaching out to Ryan for assistance from SoftVision. Ryan, as mentioned in London, it would be great to get SoftVision's help here. Please let me know what you need from me to get started on a test plan etc.
Flags: needinfo?(ryanvm)
Assignee | ||
Comment 41•8 years ago
|
||
Sending this back for one last look. Thanks, Felipe!
Attachment #8766334 -
Attachment is obsolete: true
Attachment #8768883 -
Flags: review?(felipc)
Comment 42•8 years ago
|
||
Comment on attachment 8768883 [details] [review] System addon Looks great!
Attachment #8768883 -
Flags: review?(felipc) → review+
Comment 43•8 years ago
|
||
Adding Brindusa to CC, since her team will verify the fix.
Updated•8 years ago
|
QA Contact: adrian.florinescu
Comment 44•8 years ago
|
||
(In reply to Stephen A Pohl [:spohl] from comment #38) > > I think you can test by hosting an unsigned xpi and some magic file. Rob > > Helmer knows more here, but we should get this documented, so I'll try and > > get more on that later. > > Please let me know if you'd like me to n-i Rob here or if you were just > going to get back to me. It would be great to have the ability to test this > before requesting signing. I had a chat to Rob, and found out that you can't disable the signing requirement for the download/update process. You can, of course, still turn xpinstall.signatures.required to false and install it manually - it should effectively be the same process as if it had updated (or symlink it from the profile). You could probably also use Loop's https://github.com/mozilla/loop/blob/master/bin/runfx.js which would let you easily run up FF with a profile with the add-on installed. > > Talking about testing, how about adding a functional test or something? This > > would make it easier for people thinking about deploying it to check it > > still does the expected prompts for each version. > > > > Loop is just switching to selenium+geckodriver as the supported route, > > although that's only supported from FF 48, but it may be the simpler way to > > go and give us protection for future changes. Maybe also helping to minimise > > QA. There's also taskcluster support that can easily be worked in. > > Is there documentation/examples that you could point me to? Do Loop's tests > live in its GitHub repo? I have 0 experience with selenium+geckodriver. Loop's selenium based ones are on https://github.com/mozilla/loop/tree/switch-to-selenium We currently have them running in a linux instance in taskcluster. I'm happy to help more if you want it. > > > 7. Deploy to FF44. > > > > > > Does this seem reasonable? Am I missing any steps? > > > > I think you want to also consider: > > > > - Tagging the repository + bumping version number before a release. If we're > > just shipping an existing version to a new version of FF, then it wouldn't > > need to be changed. > > I don't understand this. Which repository should I be tagging? The GitHub > repop with the addon? How do I tag a repo? Are you referring to bumping the > version number of the addon? I'm assuming the current version of 1.0 would > be sufficient for the initial release to FF44 and we'd bump it if we were to > make any changes to it? Or would we also have to bump it if we were to start > shipping it to FF45? For clarity, we want to ship this addon to FF44 at > first. For the version number, I'd start off with 1.0.0 (or whatever) as the first release. If you're shipping the same version of the add-on to another release, don't re-version, just use the same one (as long as the max version fits). If you're updating L10n or changing something in the add-on, then bump the number & re-tag. If you look at Loop's package.json, we've got a script hooked up, that allows us to run "npm version patch" or something similar, which makes bumping the version & tagging super easy: https://github.com/mozilla/loop/blob/c2279804b44a964979a74ade6383779656a9d147/package.json#L79 > > - Having minimal deployment documentation. > > Is there an example for this? Or a list of things to include? Where will the > documentation live? I would suggest, build, test & release (e.g. how to tag). I don't think you need the Go faster process, you could refer to the other docs for that.
Flags: needinfo?(standard8)
Assignee | ||
Comment 45•8 years ago
|
||
Pike, do you know if all locales have been translated already? Or should we have QA test that we properly fall back to English for locales that haven't been translated yet? Also, do you happen to know if dropping the |buttonaccesskey| from addon/outofdate-notifications@mozilla.org/locales/lv/outofdate-notifications.properties was deliberate? Thank you!
Flags: needinfo?(l10n)
Comment 46•8 years ago
|
||
You can see the current status in Pontoon https://pontoon.mozilla.org/projects/firefox-out-of-date-add-on/ or here https://l10n.mozilla-community.org/webstatus/?product=outofdate-notifications-system-addon The answer is no, we don't have all locales, and we won't have them all if we wait for more time. We currently have 46, which is a pretty good number. If you need a list JSON: https://l10n.mozilla-community.org/webstatus/api/?product=outofdate-notifications-system-addon&type=complete TXT: https://l10n.mozilla-community.org/webstatus/api/?product=outofdate-notifications-system-addon&type=complete&txt Having QA checking that the add-on is localized for a couple of major locales, and fall back to English for at least one not localized seems like a good idea. The string is missing from lv because not translated in Pontoon, I will try to see if I can find someone online to add it.
Flags: needinfo?(l10n)
Comment 47•8 years ago
|
||
(In reply to Francesco Lodolo [:flod] from comment #46) > The string is missing from lv because not translated in Pontoon, I will try > to see if I can find someone online to add it. Actually added the missing accesskey myself and sent an email to the localizer, Pontoon should update it at the beginning of the next hour.
Assignee | ||
Comment 48•8 years ago
|
||
Oh, perfect. Thank you, :flod!
Assignee | ||
Comment 49•8 years ago
|
||
(In reply to Mark Banner (:standard8) (afk until 26th July) from comment #44) > (In reply to Stephen A Pohl [:spohl] from comment #38) > > > Talking about testing, how about adding a functional test or something? This > > > would make it easier for people thinking about deploying it to check it > > > still does the expected prompts for each version. > > > > > > Loop is just switching to selenium+geckodriver as the supported route, > > > although that's only supported from FF 48, but it may be the simpler way to > > > go and give us protection for future changes. Maybe also helping to minimise > > > QA. There's also taskcluster support that can easily be worked in. > > > > Is there documentation/examples that you could point me to? Do Loop's tests > > live in its GitHub repo? I have 0 experience with selenium+geckodriver. > > Loop's selenium based ones are on > https://github.com/mozilla/loop/tree/switch-to-selenium > > We currently have them running in a linux instance in taskcluster. > > I'm happy to help more if you want it. I've had a look at this, but I get the impression that this is overkill for this simple addon. We also have dedicated testing/verification by SoftVision. Let me know if I'm mistaken. > > > > 7. Deploy to FF44. > > > > > > > > Does this seem reasonable? Am I missing any steps? > > > > > > I think you want to also consider: > > > > > > - Tagging the repository + bumping version number before a release. If we're > > > just shipping an existing version to a new version of FF, then it wouldn't > > > need to be changed. > > > > I don't understand this. Which repository should I be tagging? The GitHub > > repop with the addon? How do I tag a repo? Are you referring to bumping the > > version number of the addon? I'm assuming the current version of 1.0 would > > be sufficient for the initial release to FF44 and we'd bump it if we were to > > make any changes to it? Or would we also have to bump it if we were to start > > shipping it to FF45? For clarity, we want to ship this addon to FF44 at > > first. > > For the version number, I'd start off with 1.0.0 (or whatever) as the first > release. If you're shipping the same version of the add-on to another > release, don't re-version, just use the same one (as long as the max version > fits). If you're updating L10n or changing something in the add-on, then > bump the number & re-tag. > > If you look at Loop's package.json, we've got a script hooked up, that > allows us to run "npm version patch" or something similar, which makes > bumping the version & tagging super easy: > > https://github.com/mozilla/loop/blob/ > c2279804b44a964979a74ade6383779656a9d147/package.json#L79 I've tagged the repo with v1.0 and uploaded the .xpi for signing in bug 1287897. > > > - Having minimal deployment documentation. > > > > Is there an example for this? Or a list of things to include? Where will the > > documentation live? > > I would suggest, build, test & release (e.g. how to tag). I don't think you > need the Go faster process, you could refer to the other docs for that. Since the process is still a moving target I've added a reference to https://wiki.mozilla.org/Firefox/Go_Faster/Releasing_an_add-on_mechanics to the README.md.
Comment 50•8 years ago
|
||
Stephen poked me on IRC to get this enabled on a test channel. This addon should be available for all 44.0 versions on the "release-sysaddon" channel.
Assignee | ||
Comment 51•8 years ago
|
||
SoftVision realized that we'd be affecting partner repacks with this addon. Of the users on Firefox 44, a little less than 10% are using partner repacks. We should ignore partner repacks until we've figured out how to properly direct them to a URL where they can download the current version of their partner repack. Note: I had to choose a default string other than empty string "" when I look up the distribution.id pref because there are seemingly partner repacks with an empty string in the wild.
Attachment #8773443 -
Flags: review?(felipc)
Comment 52•8 years ago
|
||
It *might* be better to check for the existence of the distribution directory since the distribution directory can be used by non repacks for customization. I say might since I'm leaning toward getting these people on the latest version especially with the process being manual and letting them reapply the customizations.
Assignee | ||
Comment 53•8 years ago
|
||
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #52) > It *might* be better to check for the existence of the distribution > directory since the distribution directory can be used by non repacks for > customization. I say might since I'm leaning toward getting these people on > the latest version especially with the process being manual and letting them > reapply the customizations. This was the path suggested by mkaply, but I'm going to give you executive powers to decide what we want to do here. Just let me know.
Flags: needinfo?(robert.strong.bugs)
Comment 54•8 years ago
|
||
If you go with the pref route, please check using the default prefs instead of the normal prefs. Also, one question that I have is that UpdateUtils.jsm also check the "app.partner." prefs when calculating the update channel name. Would that need to be checked too or is checking "distribution.id" enough?
Comment 55•8 years ago
|
||
mkaply, considering that the users are on a version several releases behind and that they have to manually update would you be against just using the pref instead of the dir to filter out only those users?
Flags: needinfo?(robert.strong.bugs) → needinfo?(mozilla)
Comment 56•8 years ago
|
||
I don't mind if we just use the pref. Mike Connor's suggestion is that we only exclude known distributions and everything else recommend an update. Here's the list (and you would check prefixes, not just exact values). "1und1", "acer", "aol", "bing", "gmx", "mail.com", "toshiba", "webde", "yandex", "yahoo" So basically wipe the distribution directory for anything that's not official. And I agree with Felipe that you should check the default pref. As far as the app.partner prefs go, checking distribution.id should be enough. That's the primary mechanism.
Flags: needinfo?(mozilla)
Comment 57•8 years ago
|
||
mkaply, is that list generated from Stephen's report, from the partner repo, or something else? If it is from Stephen's report do you have a list or could you provide one? Stephen, how difficult will it be to only filter out the known distributions listed in comment #56 instead of everything that is a distribution? Also, if one day there is a rev2 of this add-on will there be problems with existing users of the add-on to include these distributions other than the code to support these distributions?
Flags: needinfo?(spohl.mozilla.bugs)
Flags: needinfo?(mozilla)
Comment 58•8 years ago
|
||
> mkaply, is that list generated from Stephen's report, from the partner repo, or something else? If it is from Stephen's report do you have a list or could you provide one?
Something else. It's the list we use to exclude partner distros from geo specific search changes, so it's the best list we have of current partners.
Flags: needinfo?(mozilla)
Comment 59•8 years ago
|
||
Comment on attachment 8773443 [details] [review] Ignore partner repacks clearing up the request for now due to the possible changes mentioned, and to check the default pref branch instead. example here: http://searchfox.org/mozilla-central/rev/496904277ce0143bc1a952f2eb2c7e6a81aa3d4d/toolkit/mozapps/extensions/internal/XPIProvider.jsm#2680
Attachment #8773443 -
Flags: review?(felipc)
Assignee | ||
Comment 60•8 years ago
|
||
Comment on attachment 8773443 [details] [review] Ignore partner repacks Addressed feedback, updated and rebased git pull request.
Flags: needinfo?(spohl.mozilla.bugs)
Attachment #8773443 -
Flags: review?(felipc)
Comment 61•8 years ago
|
||
Looks like the first question is answered by the new patch but what about the second question? (In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #57) > Stephen, <snip> Also, if > one day there is a rev2 of this add-on will there be problems with existing > users of the add-on to include these distributions other than the code to > support these distributions?
Flags: needinfo?(spohl.mozilla.bugs)
Assignee | ||
Comment 62•8 years ago
|
||
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #61) > Looks like the first question is answered by the new patch but what about > the second question? > > (In reply to Robert Strong [:rstrong] (use needinfo to contact me) from > comment #57) > > Stephen, <snip> Also, if > > one day there is a rev2 of this add-on will there be problems with existing > > users of the add-on to include these distributions other than the code to > > support these distributions? Existing users of the addon will be updated to rev2, asssuming we publish it for the version of Firefox that these users are on. The old addon will be removed and there will be no problems in that regard.
Flags: needinfo?(spohl.mozilla.bugs)
Updated•8 years ago
|
Attachment #8773443 -
Flags: review?(felipc) → review+
Assignee | ||
Comment 63•8 years ago
|
||
Ben, could you update the release-sysaddon channel in balrog to point to v1.2 of this addon? URL is in [1] and version is 1.2 now. Thank you! [1] https://ftp.mozilla.org/pub/system-addons/outofdate-notifications/outofdate-notifications-1.2@mozilla.org.xpi
Flags: needinfo?(bhearsum)
Comment 64•8 years ago
|
||
(In reply to Stephen A Pohl [:spohl] from comment #63) > Ben, could you update the release-sysaddon channel in balrog to point to > v1.2 of this addon? URL is in [1] and version is 1.2 now. Thank you! > > [1] > https://ftp.mozilla.org/pub/system-addons/outofdate-notifications/outofdate- > notifications-1.2@mozilla.org.xpi Done!
Flags: needinfo?(bhearsum)
Assignee | ||
Comment 65•8 years ago
|
||
Felipe, I just tested the addon but it doesn't display the yellow notification bar when it's first installed. Opening new tabs or windows also doesn't work and about:support doesn't list the addon, even though the console clearly shows that the .xpi was downloaded. Once I restart Firefox, the yellow notification bar is displayed correctly and the addon is listed in about:support under Extensions. Is this the best we can do? Or could we display the notification bar immediately after install if we were to call |startup()| from the |install()| function in bootstrap.js?
Flags: needinfo?(felipc)
Assignee | ||
Comment 66•8 years ago
|
||
(In reply to Stephen A Pohl [:spohl] from comment #65) > Opening new tabs or windows also doesn't work ^^^ Just to avoid confusion, opening new tabs or windows does work, but it doesn't display the yellow notification bar when I do so. <:-)
Comment 67•8 years ago
|
||
startup() should have been called when the extension is installed. Can you toggle the pref extensions.logging.enabled to true to see if it's being called? It will spew a lot of stuff to stdout (and browser console) and one of the things are the calls to the bootstrap.js methods.
Assignee | ||
Comment 68•8 years ago
|
||
Here is the browser console output from the session when the .xpi was first downloaded. It's probably easiest to search for the string "outofdate". I could not see a call to startup() in this log.
Assignee | ||
Comment 69•8 years ago
|
||
Here is the console output after restarting Firefox, which shows startup() being called on the new found system addon.
Comment 70•8 years ago
|
||
Hm that's a bit unexpected to me but I wouldn't be surprised if this is wired differently for system add-ons. There's a "Installing new system add-on set" in the log, but both the install() and startup() methods are only called on the next startup. Let's check with rhelmer if this is intentional or not
Flags: needinfo?(felipc) → needinfo?(rhelmer)
Comment 71•8 years ago
|
||
(In reply to :Felipe Gomes (needinfo me!) from comment #70) > Hm that's a bit unexpected to me but I wouldn't be surprised if this is > wired differently for system add-ons. There's a "Installing new system > add-on set" in the log, but both the install() and startup() methods are > only called on the next startup. > > Let's check with rhelmer if this is intentional or not Yes, system add-on updates currently require an application restart (bug 1204156, this should be easy to implement now that we have an API for delaying updates from bug 1231172).
Flags: needinfo?(rhelmer)
Comment 72•8 years ago
|
||
Ok thanks for the info. This is technically not an update, since this is a new system add-on being installed, but I imagine it's still treated as an update of the system add-ons set, and everything is delayed until the next restart?
Comment 73•8 years ago
|
||
(In reply to :Felipe Gomes (needinfo me!) from comment #72) > Ok thanks for the info. This is technically not an update, since this is a > new system add-on being installed, but I imagine it's still treated as an > update of the system add-ons set, and everything is delayed until the next > restart? Right.
Assignee | ||
Comment 74•8 years ago
|
||
:flod, the addon does not appear to use localized strings and always appears to use en-US. I've just verified this on en-US OSX 10.11 with Firefox 44.0.2 in French: Although Firefox has localized menus in French, the system addon's notification and button are in English. Is there a way to debug this? Or are you able to tell based on the directory structure[1] and/or our code[2] why this might be falling back to en-US? [1] https://github.com/mozilla/outofdate-notifications-system-addon/tree/master/addon/outofdate-notifications%40mozilla.org/locales [2] https://github.com/mozilla/outofdate-notifications-system-addon/blob/master/addon/outofdate-notifications%40mozilla.org/bootstrap.js#L13
Flags: needinfo?(francesco.lodolo)
Assignee | ||
Comment 75•8 years ago
|
||
:standard8 just pointed out to me that the locales are missing in the chrome.manifest. I was under the impression that Pontoon would take care of this, but that doesn't seem to be the case. Is the proper workflow to add them manually myself whenever all strings for a particular language have been localized?
Comment 76•8 years ago
|
||
I was going to point out the same. Pontoon doesn't update that file, you need to script it somehow. You can use this as a reference, keeping en-US https://l10n.mozilla-community.org/webstatus/api/?product=outofdate-notifications-system-addon&type=complete&txt
Flags: needinfo?(francesco.lodolo)
Comment 77•8 years ago
|
||
Note, I did point out that the chrome.manifest pieces need to be done manually, see (In reply to Axel Hecht [:Pike] from comment #20) > > 3. Localization: each locale seems to create a folder (such as en-US) with > > an outofdate-notifications.properties file, a locale-specific manifest file > > (en-US.manifest) and this manifest file is referenced in the > > chrome.manifest. Is it possible to automate the creation and addition of new > > locales such that this does not need to occur at build-time? > > We don't have anything automated for the manifest stuff. Someone will need > to do that manually, and I think it'd be easier to just do that in the > chrome.manifest, instead of doing that twice. There shouldn't be any > technical requirement to factor those out.
Assignee | ||
Comment 78•8 years ago
|
||
Yes, that was a misunderstanding on my part, sorry. I'll be working through any other issues that QA finds and will then request a new version of the addon to be signed and staged.
Assignee | ||
Comment 79•8 years ago
|
||
This has shipped about 1 hour ago. Dependent bugs that are still open are intended to be fixed in a future release.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Comment 80•8 years ago
|
||
testplan |
[Test Plan]: https://wiki.mozilla.org/QA/OutofDateNotification-SystemAddon
Updated•8 years ago
|
Whiteboard: [go-faster-system-addon]
You need to log in
before you can comment on or make changes to this bug.
Description
•