889349 - respect the add-on compat option for updates

Status: RESOLVED INVALID

(Firefox for Metro Graveyard :: Install/Update, defect, P3)

Description

[Brian R. Bondy [:bbondy]]

Metro Firefox and Desktop Firefox share the same files on disk.  Both browsers can perform updates.

Currently Metro doesn't respect the add-on option.  We'd like to respect the add-on compatibility option for Desktop's add-ons, while doing updates from Metro.

tabraldes, I think you did some work/investigation for importing add-ons.  Maybe you already have code for reading into profiles?

rstrong, this may be easier once bug 853389 and bug 853389 land?

p=13

Comment 1

[Brian R. Bondy [:bbondy]]

added "and respect the add-on compat option for updates" to the subject so this should be p=20.

Comment 2

[Jim Mathies [:jimm]]

I'm not 100% familiar with how we deal with doing updates on the metro side when desktop is running. Are we going to have issues here with trying to update addons from metro when the addon might be in use on desktop? Or do we avoid update checks when desktop is running?

Comment 3

[Brian R. Bondy [:bbondy]]

(In reply to Jim Mathies [:jimm] from comment #2)
> I'm not 100% familiar with how we deal with doing updates on the metro side
> when desktop is running. Are we going to have issues here with trying to
> update addons from metro when the addon might be in use on desktop? Or do we
> avoid update checks when desktop is running?

This isn't about updating the add-ons, but about giving a warning on Desktop when an update would create incompatible add-ons.

Comment 4

[Brian R. Bondy [:bbondy]]

If we can live with Metro breaking addons for incompatible addon Firefox updates in v1, then we can triage this to v2.

> Give Metro Firefox the ability to read Desktop Firefox's add-ons

Another option for this portion of the work is to sync the addons to registry on delayed startup when Desktop Firefox is open.  We do this for preferences, it isn't perfect but would catch most of the use cases.

> and respect the add-on compat option for updates

This portion of the work will need UX designs.

Comment 5

[Marco Mucci [:MarcoM]]

Hi Asa, please see discussion for including this feature in a V1 or V2 release.

Comment 6

[Asa Dotzler [:asa]]

(In reply to Brian R. Bondy [:bbondy] from comment #4)
> If we can live with Metro breaking addons for incompatible addon Firefox
> updates in v1, then we can triage this to v2

Can we teach Desktop's add-on manager to do the add-on compat and update on fist launch after a Metro-delivered update?  

From what I can tell, we don't actually stop the update if we find incompatible add-ons. We do the update, we just trigger a "get add-on updates too" (from what I understand).  If we could get Desktop to do that at launch after an update, we'd have a pretty clean way out of this, it seems.

Comment 7

[Brian R. Bondy [:bbondy]]

(In reply to Asa Dotzler [:asa] from comment #6)
> (In reply to Brian R. Bondy [:bbondy] from comment #4)
> > If we can live with Metro breaking addons for incompatible addon Firefox
> > updates in v1, then we can triage this to v2
> 
> Can we teach Desktop's add-on manager to do the add-on compat and update on
> fist launch after a Metro-delivered update?  
> 
> From what I can tell, we don't actually stop the update if we find
> incompatible add-ons. We do the update, we just trigger a "get add-on
> updates too" (from what I understand).  If we could get Desktop to do that
> at launch after an update, we'd have a pretty clean way out of this, it
> seems.

From what I understand we warn the user that the update will break the addons, and let them choose whether or not they want to update. If we showed them a message after the update it would be too late, their addons would already be broken in such a case.

Please confirm rstrong.

Comment 8

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

(In reply to Brian R. Bondy [:bbondy] from comment #0)
[...]
> tabraldes, I think you did some work/investigation for importing add-ons. 
> Maybe you already have code for reading into profiles?

The advice I got from other engineers is that you don't want to try to read data from a desktop Firefox profile unless you are desktop Firefox and have exclusively locked that profile.

For this use case it's hard to imagine something going terribly wrong, even if the user is adding/removing/updating addons while metro Firefox is doing an update check.  Still, I don't think there are tools that we could use (without modifying them first) to load a desktop Firefox profile into metro Firefox for enumerating the addons, especially if that profile is already locked.

Comment 9

[Robert Strong (they/them - no direct email)]

(In reply to Brian R. Bondy [:bbondy] from comment #7)
> (In reply to Asa Dotzler [:asa] from comment #6)
> > (In reply to Brian R. Bondy [:bbondy] from comment #4)
> > > If we can live with Metro breaking addons for incompatible addon Firefox
> > > updates in v1, then we can triage this to v2
> > 
> > Can we teach Desktop's add-on manager to do the add-on compat and update on
> > fist launch after a Metro-delivered update?  
> > 
> > From what I can tell, we don't actually stop the update if we find
> > incompatible add-ons. We do the update, we just trigger a "get add-on
> > updates too" (from what I understand).  If we could get Desktop to do that
> > at launch after an update, we'd have a pretty clean way out of this, it
> > seems.
> 
> From what I understand we warn the user that the update will break the
> addons, and let them choose whether or not they want to update. If we showed
> them a message after the update it would be too late, their addons would
> already be broken in such a case.
> 
> Please confirm rstrong.
Confirmed. That is an add-ons manager ui that is shown on app version change (app version greater or lesser) since there could be incompatible add-ons with the version that was changed to.

Comment 10

[Asa Dotzler [:asa]]

(In reply to Robert Strong [:rstrong] (do not email) from comment #9)
> (In reply to Brian R. Bondy [:bbondy] from comment #7)
> > (In reply to Asa Dotzler [:asa] from comment #6)
> > > (In reply to Brian R. Bondy [:bbondy] from comment #4)
> > > > If we can live with Metro breaking addons for incompatible addon Firefox
> > > > updates in v1, then we can triage this to v2
> > > 
> > > Can we teach Desktop's add-on manager to do the add-on compat and update on
> > > fist launch after a Metro-delivered update?  
> > > 
> > > From what I can tell, we don't actually stop the update if we find
> > > incompatible add-ons. We do the update, we just trigger a "get add-on
> > > updates too" (from what I understand).  If we could get Desktop to do that
> > > at launch after an update, we'd have a pretty clean way out of this, it
> > > seems.
> > 
> > From what I understand we warn the user that the update will break the
> > addons, and let them choose whether or not they want to update. If we showed
> > them a message after the update it would be too late, their addons would
> > already be broken in such a case.
> > 
> > Please confirm rstrong.
> Confirmed. That is an add-ons manager ui that is shown on app version change
> (app version greater or lesser) since there could be incompatible add-ons
> with the version that was changed to.

Sorry, I seem to have confused Updater and Add-on Manager functionality. 

As long as the Add-on manager still does its thing on Desktop after a user does an update from Metro, I'm OK.

We can kill the incompat add-on warning in the updater, across the board, IMO, so long as Desktop knows to try to fetch updates for incompatible add-ons when it is next launched.

Comment 11

[Robert Strong (they/them - no direct email)]

(In reply to Asa Dotzler [:asa] from comment #10)
> (In reply to Robert Strong [:rstrong] (do not email) from comment #9)
> > (In reply to Brian R. Bondy [:bbondy] from comment #7)
> > > (In reply to Asa Dotzler [:asa] from comment #6)
> > > > (In reply to Brian R. Bondy [:bbondy] from comment #4)
> > > > > If we can live with Metro breaking addons for incompatible addon Firefox
> > > > > updates in v1, then we can triage this to v2
> > > > 
> > > > Can we teach Desktop's add-on manager to do the add-on compat and update on
> > > > fist launch after a Metro-delivered update?  
> > > > 
> > > > From what I can tell, we don't actually stop the update if we find
> > > > incompatible add-ons. We do the update, we just trigger a "get add-on
> > > > updates too" (from what I understand).  If we could get Desktop to do that
> > > > at launch after an update, we'd have a pretty clean way out of this, it
> > > > seems.
> > > 
> > > From what I understand we warn the user that the update will break the
> > > addons, and let them choose whether or not they want to update. If we showed
> > > them a message after the update it would be too late, their addons would
> > > already be broken in such a case.
> > > 
> > > Please confirm rstrong.
> > Confirmed. That is an add-ons manager ui that is shown on app version change
> > (app version greater or lesser) since there could be incompatible add-ons
> > with the version that was changed to.
> 
> Sorry, I seem to have confused Updater and Add-on Manager functionality. 
> 
> As long as the Add-on manager still does its thing on Desktop after a user
> does an update from Metro, I'm OK.
> 
> We can kill the incompat add-on warning in the updater, across the board,
> IMO, so long as Desktop knows to try to fetch updates for incompatible
> add-ons when it is next launched.
If you'd like this change please discuss it with Gavin / send an email to firefox-dev.

Comment 12

[Asa Dotzler [:asa]]

I think we should make this a wontfix or push out to Metro v2. 

I also think we should file a new bug on removing this check from the updater for all of our platforms.  We don't support previous versions any more I don't think we're doing users or the web a favor by encouraging users to stay on software with known exploits. Let's kill the updater's knowledge of add-ons completely. I'll follow up on that separately but for Metro we can close the issue out with the solution we have which is to disable the add-on compat check on Windows 8.

Comment 13

[Robert Strong (they/them - no direct email)]

(In reply to Asa Dotzler [:asa] from comment #12)
> I think we should make this a wontfix or push out to Metro v2. 
> 
> I also think we should file a new bug on removing this check from the
> updater for all of our platforms.
Note: this is essentially the way it was previously which bug 889039 reverted.

Please discuss this with Gavin / firefox-dev mail list.

BTW: I am all for this change since it will almost definitely help uptake rate.

>  We don't support previous versions any
> more
I believe we do and this is the first I heard of this.

> I don't think we're doing users or the web a favor by encouraging users
> to stay on software with known exploits. Let's kill the updater's knowledge
> of add-ons completely.
If there is buy in for this change, for the initial implementation just flip the Firefox preference and remove the UI for this in the Firefox Options panel. After that we can evaluate whether it is even necessary to remove it from app update.

> I'll follow up on that separately but for Metro we
> can close the issue out with the solution we have which is to disable the
> add-on compat check on Windows 8.
So for clarity, the pref will be changed only on Win8 and will stay the same for other platforms and Windows version?

Comment 14

[Asa Dotzler [:asa]]

(In reply to Robert Strong [:rstrong] (do not email) from comment #13)
> (In reply to Asa Dotzler [:asa] from comment #12)
> > I think we should make this a wontfix or push out to Metro v2. 
> > 
> > I also think we should file a new bug on removing this check from the
> > updater for all of our platforms.
> Note: this is essentially the way it was previously which bug 889039
> reverted.

Yes. I'm sorry. I've made a bit of a mess here because I didn't understand which parts of the add-on negotiation was being performed by the Updater and which parts were being done by the Firefox Add-ons manager. 

I think we had this basically right, disabling add-on checking for Windows 8, before I got worried that we weren't going to "automatically" check for updates for broken add-ons for our users.  Now that I realize that this happens in the Firefox app's add-on Manager after an update (or downgrade) I'm no longer concerned about the part of the check that happens in the updater.
 
> Please discuss this with Gavin / firefox-dev mail list.
> 
> BTW: I am all for this change since it will almost definitely help uptake
> rate.

I think we do this for Windows 8 because that's the urgent need and then I take the proposal to make this for all versions to dev.firefox for further discussion. 

> >  We don't support previous versions any
> > more
> I believe we do and this is the first I heard of this.

We stopped supporting older versions with the move to the faster release cadence with Firefox 4.0.  Helping users stay on less than current versions of Firefox which will *not* receive security updates (my definition of "support") is helping our users get hacked. We should stop doing that, IMO. That will be the discussion to have with a broader set of stakeholders, I think.

> > I don't think we're doing users or the web a favor by encouraging users
> > to stay on software with known exploits. Let's kill the updater's knowledge
> > of add-ons completely.
> If there is buy in for this change, for the initial implementation just flip
> the Firefox preference and remove the UI for this in the Firefox Options
> panel. After that we can evaluate whether it is even necessary to remove it
> from app update.

Glad it's an easy one to make happen.

> > I'll follow up on that separately but for Metro we
> > can close the issue out with the solution we have which is to disable the
> > add-on compat check on Windows 8.
> So for clarity, the pref will be changed only on Win8 and will stay the same
> for other platforms and Windows version?

Yes. For now. (This is assuming we are all OK doing it for Windows 8 as we were before I got involved and confused a bunch of things.)

Comment 15

[Brian R. Bondy [:bbondy]]

... (snip)
> This is assuming we are all OK doing it for Windows 8

Comment 16

[:Gavin Sharp [email: [e-mail]]]

What's the "urgent need" referred to in comment 14?

As I understand it, if we don't make any changes, updates on the Metro side will potentially lead to incompatible add-ons in Desktop after a restart. That's inconsistent with the update process on the Desktop side (which notifies the user of incompatibilities first), but given that we seem to be OK with moving in that direction for all updates, that inconsistency doesn't seem important to solve.

What does disabling the add-on compatibility notification on Windows 8 only get us? Are there issues with "doing nothing" that I'm missing?

Comment 17

[Brian R. Bondy [:bbondy]]

I think that if a user goes into preferences and sees a checkbox ON that says don't let updates break my add-ons, then we shouldn't break their adddons. Otherwise we're knowingly misleading and lying to users. 

If we default it off then it is not a bug, and that's what the current UI in Desktop options in Win8 is modeled after.

Comment 18

[Brian R. Bondy [:bbondy]]

For the current UI in Win8 Desktop options by the way, to turn the option off, you have to also disable Metro updates. 

In the current UI a user will never be misled into thinking it does something that it doesn't, and I think that's a good thing.

Comment 19

[Brian R. Bondy [:bbondy]]

Attachment: Current UI for Win8 Desktop options

Comment 20

[Brian R. Bondy [:bbondy]]

Talked to gavin on IRC, he is suggesting that we:
a) Leave the UI the same (see the attachment)
b) Leave the checkbox off and disabled when metro is selected
c) Keep the preference on when metro is selected
d) Continue to ignore that preference in Metro either way. 

That means that there is a small code change needed to unhook the checkbox from the pref value in the case of metro updates. In that case we'll show it off but actually have it forced on for Desktop but not Metro.

If we want to go ahead with this plan I'll file a new bug because I think this bug's title is the ideal way to move forward later.

needinfo'ing to ensure we're all ok with this plan before I make this change. Thanks :)

Comment 21

[Robert Strong (they/them - no direct email)]

(In reply to Brian R. Bondy [:bbondy] from comment #20)
> Talked to gavin on IRC, he is suggesting that we:
> a) Leave the UI the same (see the attachment)
> b) Leave the checkbox off and disabled when metro is selected
> c) Keep the preference on when metro is selected
Which preference?

> d) Continue to ignore that preference in Metro either way. 
>
> That means that there is a small code change needed to unhook the checkbox
> from the pref value in the case of metro updates. In that case we'll show it
> off but actually have it forced on for Desktop but not Metro.
Does "forced on in Desktop" mean that the user can't disable the incompatible add-on check in desktop?

> If we want to go ahead with this plan I'll file a new bug because I think
> this bug's title is the ideal way to move forward later.
If this capability is the end result in the future then I'm less concerned about the what is implemented in the interim.

> needinfo'ing to ensure we're all ok with this plan before I make this
> change. Thanks :)

Comment 22

[Brian R. Bondy [:bbondy]]

(In reply to Robert Strong [:rstrong] (do not email) from comment #21)
> (In reply to Brian R. Bondy [:bbondy] from comment #20)
> > Talked to gavin on IRC, he is suggesting that we:
> > a) Leave the UI the same (see the attachment)
> > b) Leave the checkbox off and disabled when metro is selected
> > c) Keep the preference on when metro is selected
> Which preference?

The one corresponding to add-on compat: app.update.mode


> > d) Continue to ignore that preference in Metro either way. 
> >
> > That means that there is a small code change needed to unhook the checkbox
> > from the pref value in the case of metro updates. In that case we'll show it
> > off but actually have it forced on for Desktop but not Metro.
> Does "forced on in Desktop" mean that the user can't disable the
> incompatible add-on check in desktop?

From what I understand, and please correct me gavin if I'm wrong, gavin wants 0 changes to UI. 

> [15:09:20] <gavin> I'm suggesting doing absolutely nothing as of now

i) Meaning if Metro+Desktop updates are selected, the checkbox is off and disabled, but the add-on compat preference value is forced on
ii) Meaning if Desktop updates only is selected, the user can turn the checkbox on or off and the add-on compat preference value will change accordingly.

Comment 23

[Robert Strong (they/them - no direct email)]

I don't think that is as good as the other option presented since people won't be able to disable the check if they want but afaic it is Gavin's call.

Comment 24

[:Gavin Sharp [email: [e-mail]]]

(In reply to Robert Strong [:rstrong] (do not email) from comment #23)
> I don't think that is as good as the other option presented since people
> won't be able to disable the check if they want but afaic it is Gavin's call.

"the other option presented" being change the default app.update.mode and don't allow it to be toggled, on Windows 8 only?

I don't like that because it introduces an inconsistency in our default update behavior across platforms. In practice I think the set of people who change these options from their defaults is relatively small, so losing the ability to "opt out" of the add-on compatibility warning on Windows 8 Desktop if you're updating Metro+Desktop doesn't seem so bad in comparison, and isolates the weirdness to Metro.

I would support making a UI tweak per comment 20:
> That means that there is a small code change needed to unhook the checkbox from
> the pref value in the case of metro updates. In that case we'll show it off but
> actually have it forced on for Desktop but not Metro.

I also support investigating removing the add-on compat warning across the board, but that should probably be decoupled from any time-sensitive work.

Comment 25

[Robert Strong (they/them - no direct email)]

(In reply to :Gavin Sharp (use [e-mail] for email) from comment #24)
> (In reply to Robert Strong [:rstrong] (do not email) from comment #23)
> > I don't think that is as good as the other option presented since people
> > won't be able to disable the check if they want but afaic it is Gavin's call.
> 
> "the other option presented" being change the default app.update.mode and
> don't allow it to be toggled, on Windows 8 only?
I prefer to limiting this to Metro and respecting the pref as set by the user in the options panel though I do see your point. Another option I prefer is launching Desktop to perform the update when an update is advertised to Metro. The best option of course would be to be able to read the add-ons in the Desktop profile from Metro and I am fine with whatever solution as long as that is essentially the end result.

> I don't like that because it introduces an inconsistency in our default
> update behavior across platforms. In practice I think the set of people who
> change these options from their defaults is relatively small, so losing the
> ability to "opt out" of the add-on compatibility warning on Windows 8
> Desktop if you're updating Metro+Desktop doesn't seem so bad in comparison,
> and isolates the weirdness to Metro.
> 
> I would support making a UI tweak per comment 20:
> > That means that there is a small code change needed to unhook the checkbox from
> > the pref value in the case of metro updates. In that case we'll show it off but
> > actually have it forced on for Desktop but not Metro.
> 
> I also support investigating removing the add-on compat warning across the
> board, but that should probably be decoupled from any time-sensitive work.

Comment 26

[Brian R. Bondy [:bbondy]]

Bug 890626 was filed to do the immediate action Gavin discussed in this bug.  Leaving this bug open for the optimal plan of reading desktop's addons from Metro.