889039 - Change - Re-enable add-on compat check by default when doing updates through Firefox

Status: RESOLVED FIXED

(Firefox :: General, defect, P2)

Description

[Brian R. Bondy [:bbondy]]

Based on recent discussion it looks like we don't want to disable add-on compat update option by default.

This is a bug for a pref-only change that can be uplifted to aurora. 

The benefit here is that we can fix the default everywhere, and since Metro isn't enabled the UI won't be wrong.

Comment 1

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

I believe this should be in the Firefox component since the only code that should need to be touched is in Firefox. Do other apps need this as well?

Comment 2

[Brian R. Bondy [:bbondy]]

Attachment: Patch v1.

As discussed in the email I just sent, I'm not sure if this really matters since I think the option only has to do with major updates and almost all updates are major now.  Minor updates probably don't break addon compat ever anyway.

Comment 3

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

Comment on attachment 769807 [details] [diff] [review]
Patch v1.

Let's wait on Asa's decision on how to move forward.

Comment 4

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

As discussed via email, "Might not be a bad thing to file a bug to clean that up though I would prefer if it pointed to app update comments since it is authoritative and the comments have gotten out of date on numerous occasions."

Comment 5

[Brian R. Bondy [:bbondy]]

I'll clean up the comments once we (likely) revive this patch pending Asa's determination.

For anyone following along, Comment 2 is not valid anywhere. I was reading comments in the profile's default preferences which no longer applies.

Comment 6

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

Comment on attachment 769807 [details] [diff] [review]
Patch v1.

I just noticed the last sentence of comment #0.

I'm fine with backing out the change that was previously agreed to and covering the rest in another bug after a new decision is made.

Comment 7

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

For now, remove those comments on checkin

Comment 8

[Brian R. Bondy [:bbondy]]

Ya I think that's best since this change makes it so it works the same as it used to, then we can change from there based on new feedback.

Comment 9

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

BTW: those comments I believe were from way back when and incompatibility checking didn't even work so the comment was incorrect even when it was written which was way before my time.

Comment 10

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

(In reply to Brian R. Bondy [:bbondy] from comment #0)
> Based on recent discussion it looks like we don't want to disable add-on
> compat update option by default.

Where did this discussion take place?

I'm a bit surprised that this is the first I've heard of bug 866229, and that these changes weren't discussed on firefox-dev.

Comment 11

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

Over vidyo with Asa, Brian, Juan, and myself. I sent an email a few days ago to Asa asking him to send an email to the mail lists so devs and the community knew about this change and this is the end result.

Comment 12

[Brian R. Bondy [:bbondy]]

There is also some discussion in bug 866229 if you're interested. As noted in reply to your ping on IRC, I'll CC you on future bugs like bug 866229 that change Firefox app defaults.

Comment 13

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

This should also land on aurora
http://mxr.mozilla.org/mozilla-aurora/source/browser/app/profile/firefox.js#148

Comment 14

[Brian R. Bondy [:bbondy]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b6bca6113b5d

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b6bca6113b5d

Comment 16

[Brian R. Bondy [:bbondy]]

Comment on attachment 769807 [details] [diff] [review]
Patch v1.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 866229
User impact if declined: User's default will be changed to not warn about addon compat before updates.
Testing completed (on m-c, etc.): no, but it makes it the same as it used to be.
Risk to taking this patch (and alternatives if risky): Very low
String or IDL/UUID changes made by this patch: None.

We may end up disabling this again but it'll be done in a different but and we're not ready for this decision on aurora yet.

Comment 17

[Alex Keybl [:akeybl]]

Comment on attachment 769807 [details] [diff] [review]
Patch v1.

Just a backout of https://bug866229.bugzilla.mozilla.org/attachment.cgi?id=753374 to return to FF23 behavior.

Comment 18

[Brian R. Bondy [:bbondy]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/77ba223a21e2

Comment 19

[Marco Mucci [:MarcoM]]

Temporarily reopening to add to Metro Iteration #10.  Will be marked Resolved/Fixed once it has been added.

Comment 20

[Samvedana (:Samvedana)]

Attachment: Firefox options>advanced>update

Tested on latest nightly from ftp://ftp.mozilla.org/pub/firefox/nightly/2013/07/2013-07-17-03-02-07-mozilla-central/
using Windows 8.1 preview. I see attached options.

For second option "Automatically install updates from desktop nightly" it checks add-on compatibility.

For first option "Automatically update from desktop and Windows 8 style Nightly", "warn me if this will disable any of my add-ons" option remains selected.

Still I am not sure, This is enough for testing or not.

Can you please provide more detail to test this?

Comment 21

[Brian R. Bondy [:bbondy]]

I put the test cases in bug 890626, those will be enough.

Comment 22

[Samvedana (:Samvedana)]

WFM.
Tested on Windows 8 for iteration 10 using latest nightly from ftp://ftp.mozilla.org/pub/firefox/nightly/2013/07/2013-07-18-03-02-01-mozilla-central/

For clean profile app.update.mode was set to 1.
Options | Advanced | Update
The checkbox Warn me if this will disable any of my add-ons was checked on.
Followed steps given in comment 3 of bug 890626, checkbox for add-on compat was checked on.

Comment 23

[Cornel Ionce [:noni] [Hubs QA]]

Verified on latest Nightly using steps from comment 6, 7 and 8 of bug 890626.
The results were as expected.

In about:config the app.update.mode is set to 1.
The checkbox Warn me if this will disable any of my add-ons is checked on (Options | Advanced | Update).
For bug 890626 comment 8, the disabled checkbox for add-on compat was checked on.