Last Comment Bug 694595 - Some settings for add-ons aren't being migrated when we change database schemas
: Some settings for add-ons aren't being migrated when we change database schemas
Status: RESOLVED FIXED
[qa+]
: dataloss
Product: Toolkit
Classification: Components
Component: Add-ons Manager (show other bugs)
: 8 Branch
: All All
: -- normal (vote)
: mozilla10
Assigned To: Dave Townsend [:mossop]
:
Mentors:
Depends on: 705631
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-14 10:06 PDT by Dave Townsend [:mossop]
Modified: 2011-12-16 02:34 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed


Attachments
patch rev 1 (16.72 KB, patch)
2011-10-18 09:45 PDT, Dave Townsend [:mossop]
bmcbride: review+
christian: approval‑mozilla‑aurora+
christian: approval‑mozilla‑beta+
Details | Diff | Review

Description Dave Townsend [:mossop] 2011-10-14 10:06:20 PDT
applyBackgroundUpdates, sourceURI and resourceURI are getting lost whenever we update the DB schema
Comment 1 Dave Townsend [:mossop] 2011-10-18 09:45:25 PDT
Created attachment 567776 [details] [diff] [review]
patch rev 1

Add migration for these fields and a test for it.
Comment 2 Blair McBride [:Unfocused] (mostly unavailable, needinfo open, reviews not) 2011-10-18 17:28:35 PDT
Comment on attachment 567776 [details] [diff] [review]
patch rev 1

Review of attachment 567776 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.

(Oh hell, I'm bitrotten already)
Comment 4 Marco Bonardo [::mak] 2011-10-21 02:17:01 PDT
https://hg.mozilla.org/mozilla-central/rev/2739199a5bbd
Comment 5 Dave Townsend [:mossop] 2011-10-23 11:13:30 PDT
Comment on attachment 567776 [details] [diff] [review]
patch rev 1

Bug 693743 does a schema change which will cause this set of dataloss to happen again so I'd very much like to take this patch as well (also the patch in bug 693743 will have to be changed without it), it is low risk enough that I think it's ok to take on beta.
Comment 6 christian 2011-10-25 20:12:43 PDT
---------------------------------[ Triage Comment ]---------------------------------

Approving for 8beta and 9aurora as this is a dataloss bug that will be triggered again by bug 693743. The patch looks to be low risk and contained.

Please land ASAP.
Comment 8 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-11-21 17:42:59 PST
Are there any specific instructions for QA to verify this fix?
Comment 9 Dave Townsend [:mossop] 2011-11-22 11:51:51 PST
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #8)
> Are there any specific instructions for QA to verify this fix?

Run Firefox 7 and install an add-on, in its details change its auto-update setting to something other than default. Update to Firefox 8 and check that that setting is retained.
Comment 10 Paul Silaghi, QA [:pauly] 2011-11-25 05:08:04 PST
I've verified this on Win 7 32 and 64 updating Firefox 7.0.1 with Adblock Plus 1.3.10 to Firefox 8.0.1. The addon was initially set with automatic updates ON, but after the update has changed to OFF. If automatic updates is set to default or OFF, the setting remains the same after the update.
Comment 11 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-11-25 07:39:52 PST
I take it this bug can be marked VERIFIED then? Paul, Dave, please confirm.
Comment 12 Paul Silaghi, QA [:pauly] 2011-11-25 10:58:20 PST
No, it is not the right scenario. Dave?
Comment 13 Dave Townsend [:mossop] 2011-11-25 17:12:15 PST
(In reply to Paul Silaghi [QA] from comment #12)
> No, it is not the right scenario. Dave?

No this is not correct, please file a new bug with specific STR
Comment 14 Paul Silaghi, QA [:pauly] 2011-11-29 01:30:16 PST
New bug 705631 logged. Can I mark this as verified fixed now ?
Comment 15 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-12-13 13:49:10 PST
Dave, can this be marked verified fixed based on Paul's testing? Is there something else we can do? Should this be retested given the dependency being fixed?
Comment 16 Dave Townsend [:mossop] 2011-12-13 14:39:17 PST
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #15)
> Dave, can this be marked verified fixed based on Paul's testing? Is there
> something else we can do? Should this be retested given the dependency being
> fixed?

I would re-test with the fix from the other bug too and then verify.
Comment 17 Mihaela Velimiroviciu (:mihaelav) 2011-12-14 23:24:29 PST
Fix for bug #705631 was pushed only for m-c and cannot be verified in beta.
Comment 18 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-12-15 07:48:50 PST
Dave, will the fix for bug 705631 ever make it into Firefox 9? If not, I don't think we will be able to retest this as requested in comment 16.
Comment 19 Dave Townsend [:mossop] 2011-12-15 09:31:14 PST
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #18)
> Dave, will the fix for bug 705631 ever make it into Firefox 9? If not, I
> don't think we will be able to retest this as requested in comment 16.

You can re-test with trunk though right? I'm not sure it's worth trying to push the fix for this down to beta.
Comment 20 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-12-15 10:48:11 PST
Well we can, Dave. Though this bug was fixed on Firefox 8 and 9 -- that's where we are trying to verify the fix. Testing on trunk is fine for bug 705631, but I don't see how that enables us to mark this bug VERIFIED for 8 and 9.
Comment 21 Dave Townsend [:mossop] 2011-12-15 10:51:11 PST
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #20)
> Well we can, Dave. Though this bug was fixed on Firefox 8 and 9 -- that's
> where we are trying to verify the fix. Testing on trunk is fine for bug
> 705631, but I don't see how that enables us to mark this bug VERIFIED for 8
> and 9.

Well I guess the point is that it isn't fully fixed for 8 and 9.
Comment 22 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-12-15 11:04:12 PST
Since we already verified this once, I'm thinking we can mark this VERIFIED and we can retest it on bug 705631 if/when it gets ported to Aurora/Beta. Does this sound reasonable?
Comment 23 Mihaela Velimiroviciu (:mihaelav) 2011-12-16 02:34:51 PST
I verified the fix of bug #705631 on Nightly and it worked as expected.

Note You need to log in before you can comment on or make changes to this bug.