Last Comment Bug 630575 - Properly distinguish soft and hard blocked items in the update pings
: Properly distinguish soft and hard blocked items in the update pings
Status: RESOLVED FIXED
[has patch]
:
Product: Toolkit
Classification: Components
Component: Add-ons Manager (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla2.0b12
Assigned To: Dave Townsend [:mossop]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-02-01 10:00 PST by Dave Townsend [:mossop]
Modified: 2011-03-23 11:29 PDT (History)
3 users (show)
dtownsend: in‑testsuite+
dtownsend: in‑litmus-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
.17-fixed
.19-fixed


Attachments
patch rev 1 (7.42 KB, patch)
2011-02-09 16:59 PST, Dave Townsend [:mossop]
blair: review+
gavin.sharp: approval2.0+
Details | Diff | Splinter Review
branch patch (8.17 KB, patch)
2011-03-07 11:59 PST, Dave Townsend [:mossop]
robert.strong.bugs: review+
dveditz: approval1.9.2.17+
dveditz: approval1.9.1.19+
Details | Diff | Splinter Review

Description Dave Townsend [:mossop] 2011-02-01 10:00:08 PST
Currently in Firefox 3.5 and 3.6 we only include the "blocklisted" state if an item is hardblocked.
In Firefox 4.0 we include the "blocklisted" state if an item is soft or hard blocked.

This causes confusing data and for the branches makes it impossible to verify that blocklist changes are having any effect. We should split it and send "blocklisted" for hard blocks only and "softblocked" for soft blocked items.
Comment 1 Dave Townsend [:mossop] 2011-02-09 16:59:02 PST
Created attachment 511242 [details] [diff] [review]
patch rev 1

Just for Firefox 4 for now this makes the changes. Turns out there were no tests for this so I added those. I think it's important to take this and bring us back into parity with 3.6 for stats.
Comment 2 Blair McBride [:Unfocused] (UNAVAILABLE) 2011-02-10 01:32:50 PST
Comment on attachment 511242 [details] [diff] [review]
patch rev 1

Makes sense.
Comment 3 Dave Townsend [:mossop] 2011-02-10 10:25:37 PST
For drivers, this is really safe code that fixes a regression from 3.6 that generates us confusing stats when we try to evaluate the pickup rate of the blocklist.
Comment 4 Dave Townsend [:mossop] 2011-02-10 10:35:56 PST
Landed: http://hg.mozilla.org/mozilla-central/rev/53d06f07a4d2
Comment 5 Henrik Skupin (:whimboo) 2011-02-11 19:19:25 PST
Dave, what's the best way to verify this fix at least for the softblocked items? In which query will the softblocked items be contained in?
Comment 6 Dave Townsend [:mossop] 2011-02-15 09:41:07 PST
(In reply to comment #5)
> Dave, what's the best way to verify this fix at least for the softblocked
> items? In which query will the softblocked items be contained in?

It shows up in the update ping to AMO or the %ITEM_STATUS% part of a custom updateURL
Comment 7 Dave Townsend [:mossop] 2011-03-07 11:59:14 PST
Created attachment 517484 [details] [diff] [review]
branch patch

This adds the softblocked status to the 1.9.2 branch and adds the same tests.
Comment 8 Dave Townsend [:mossop] 2011-03-07 12:45:47 PST
Comment on attachment 517484 [details] [diff] [review]
branch patch

I'd like approval to land this on the branches to ensure that we get good feedback from changes to the blocklist. It is a pretty low risk patch and adds more tests than we had previously for this status reporting.
Comment 9 Daniel Veditz [:dveditz] 2011-03-14 10:27:19 PDT
(In reply to comment #3)
> For drivers, this is really safe code that fixes a regression from 3.6 that

If it's a regression from 3.6 why do we need this patch in 3.6? Or if you meant a regression introduced _in_ 3.6 it's still not clear why you want this patch in 3.5.
Comment 10 Dave Townsend [:mossop] 2011-03-14 11:51:03 PDT
(In reply to comment #9)
> (In reply to comment #3)
> > For drivers, this is really safe code that fixes a regression from 3.6 that
> 
> If it's a regression from 3.6 why do we need this patch in 3.6? Or if you meant
> a regression introduced _in_ 3.6 it's still not clear why you want this patch
> in 3.5.

Neither, that comment was for the 2.0 drivers where we changed the behaviour.

Firefox 3.6 and 3.5 tell AMO when they know an add-on that a user has installed is hard blocked but not when it is soft blocked (which 4.0 does do now). We have been using the stats from these pings in the past to verify the rollout of the blocklist updates and without the soft blocked information we can't verify that kind of block is rolling out as we expect.
Comment 11 Daniel Veditz [:dveditz] 2011-03-16 10:44:41 PDT
Comment on attachment 517484 [details] [diff] [review]
branch patch

Approved for 1.9.2.16 and 1.9.1.18, a=dveditz for release-drivers

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