Closed Bug 420356 Opened 13 years ago Closed 13 years ago
.xml with Firefox
I find myself wondering whether it would be a good idea to ship a base blocklist file with Firefox. Currently if you are a first time user starting Firefox there will be no blocklist present until you have been running Firefox for a continuous 10 minutes. It seems to me that that is ample time for already installed addon to crash you or for you to install a bad one. This could be as simple as including a blocklist.xml in the default profile so it'd be there for newly created profiles. People migrating from Firefox 2 already have an update to date blocklist in their profile. So long as we remember to do an update of the blocklist before each main release, the average user could be better protected for that first startup experience.
I have to correct myself. The blocklist is not downloaded until at least 1 day after the first run of Firefox.
Blocking for resolution on this; I'm curious to know what we'd populate it with? Would we update it when shipping dot-dot release updates? That sounds like goodness, too.
Flags: blocking-firefox3? → blocking-firefox3+
Priority: -- → P2
Yes my theory was that at some point during each release (maybe as we freeze or something or bump versions) we would take the current blocklist.xml and put it in for the release.
What I think we want to do here is: * Add blocklist.xml to the set of profile defaults * Tweak the "required files" code to replace if the file date is newer (optional, but probably right, consider the case where the user hasn't run firefox in a while but downloads Firefox 3 to take a spin. * Figure out the best way to get latest-and-greatest blocklist.xml into the build process. Could be as simple as a smart wget call during the build process. Not sure at all how we'd want to do this, ccing build folks for ideas.
We currently use a different url for downloading the blocklist depending on the application and version. I'm assuming that in reality there is just a single blocklist file that we can use for our purposes here though, can you confirm morgamic?
Any reason we can't just have this live in CVS?
Ted: define have it live in CVS? Meaning we'd maintain the blocklist as a CVS file and import it to the web service we're pointing at? Pretty sure we have code to generate the blocklist snippets so we don't have to push any more data than needed (IOW we should only push blocklist data that applies to the current version, anything more is wasted bandwidth and diskspace). If we had a bunch of "broken in Fx2, fine in Fx3" extensions, that single file would just add to startup time parsing, in any case. Why not just grab the appropriate blocklist for the app+version we're building?
Either have the source data in CVS, or just a snapshot of the data stored in CVS that we can update. Pre-processing it to only have data relevant to the branch it's on would be fine. Just thinking that would simplify the build logic a lot.
Having a build process do "wget" is a non-starter, because it leads to non-reproducable builds and depends on whatever machine is doing the build having wget and access to the internet... most build machines are firewalled from the internet.
(In reply to comment #4) > * Add blocklist.xml to the set of profile defaults > * Tweak the "required files" code to replace if the file date is newer > (optional, but probably right, consider the case where the user hasn't run > firefox in a while but downloads Firefox 3 to take a spin. Actually it need not necessarily be in the profile default. The blocklist service could be adjusted to look in two different places for the blocklist file, the profile and somewhere in the appdir. It could either use the newest one, or maybe even combine the two.
Agree with comment #9. In a perfect world * the info would be in CVS and end up in the default profile/appdir * updating the information in the AMO db would cause a checkin to CVS (might be able to do something similar to what egg does with configure.in)
I am actually against shipping a default blocklist.xml for two reasons: 1) user will update this via service anyway 2) we can't take it back if we ship it this way... It sucks that it isn't processed or updated for 24 hours -- what is the reason/cause for the delay? Don't want new users to see "your crap is broken" I imagine... but remember that even if you ship with Firefox that'll be the case. So why not remove the 24 hour window and update on first run or update?
(In reply to comment #12) > It sucks that it isn't processed or updated for 24 hours -- what is the > reason/cause for the delay? Don't want new users to see "your crap is broken" > I imagine... but remember that even if you ship with Firefox that'll be the > case. It's mainly a result of the original timer service being written for software update. It was decided that an automatic update wouldn't be offered run for at least a day after you ran firefox to avoid the "ran firefox and immediately I needed to update" experience. The timer service was then used by lots of other components but with the same basic behaviour the app update scenario needed. > So why not remove the 24 hour window and update on first run or update? It is a possibility sure. However to actually achieve the same effect as shipping the blocklist you would have to delay the startup until the blocklist was retrieved and processed. And you can potentially be running the app without network access at the start.
> 2) we can't take it back if we ship it this way... How about grabbing the blacklist from the server IFF something in the local blacklist matches? Or we could just say that our strategy for "taking it back" is to ship a new point release of Firefox.
(In reply to comment #12) > 2) we can't take it back if we ship it this way... We can't take it back, but it will get replaced with a new version from the server within around a day.
True -- that makes it more reasonable to me so I'm out of objections.
This implements what would be necessary for the application code. It makes the blocklist check two locations for an existing file, the profile and the application directory. Whichever is newest, if both exist, wins. This doesn't handle the issue of getting the latest blocklist for the build, for now I have just included the latest blocklist to be added. We could ship b5 with it like this and then bring in some update process for the blocklist in tree at a later date. This works for the case of installations using the installer, OSX dmg or tarball all of which preserve the time of the original build (i.e. older than the install date). It isn't ideal for the auto-update case which sets the modification time of the new file to be the time of the update. This will mean that if you auto-update then you would end up using the blocklist shipped in the update instead of the file in the profile which could be newer, until the next time the blocklist is retrieved from the server. There isn't an easy way around that without encoding the time into the app's blocklist file and parsing it to find it which would be costly. Thoughts?
Assignee: nobody → dtownsend
Status: NEW → ASSIGNED
Nick says that egg auto checks in a new configure based on these steps: "it checks out configure.in, creates configure, does a cvs diff on that, and does a checkin on a diff" This is pretty much ideal for keeping an in-CVS blocklist file up to date, just checkout the blocklist from the tree, use wget to pull a new copy and the diff and checkin as necessary.
Nick, how do we feel about this? I'd like to get this for B5 as it would help us determine if we can actually stop a lot of our initial crashing by applying some hot-hot blocking action right up front. Not sure it blocks the beta, though :(
beltzner: the automated-blocklist-in-CVS stuff shouldn't block the beta, should it? That will just make sure we're shipping the latest blocklist in future releases. Mossop's patch has the existing blocklist in it, so that should be enough for b5. (Incidentally, I bet it would be pretty easy to adapt the configure-update script to do the blocklist.)
Yeah there is no need to block b5 on automating the blocklist checkin. This patch is all that is necessary.
I think this has to block the beta: we can't be targetting RC1 for things that need in-the-field validation; RC1 should be "we think this is good to go, prove otherwise", not "we're holding our breath on the blocklist thing". But really then we would want 2 more betas, so that we can test shipping blocklist rev1 with the installer, rev2 over the wire, and rev3 (or rev1 again!) with the installer. Given that we're not going to do that, and that I don't think nightlies give us the test coverage we want, I think that means that we have to have this in b5, and then push an update over the wire, and then push an update via the update-installer. Timestamp comparisons also make me extremely nervous. Restoring backups by dragging and dropping, mishaps with clocks that put the file stamp deep into the future -- all sorts of things can make the timestamp on the disk not match our expectations of meaning. If we're going to put a timestamp in play, we should put it in the file, and we should compare it with Last-Modified from the HTTP response to guard against a bug in the update script adding a zero or otherwise promoting it to the deep future.
This is a slightly different version that only uses the application shipped blocklist if there is no blocklist in the profile.
Attachment #310358 - Flags: review?(gavin.sharp)
Attachment #310358 - Flags: review?(gavin.sharp) → review+
Checking in toolkit/mozapps/extensions/src/nsBlocklistService.js; /cvsroot/mozilla/toolkit/mozapps/extensions/src/nsBlocklistService.js,v <-- nsBlocklistService.js new revision: 1.10; previous revision: 1.9 done Checking in browser/app/Makefile.in; /cvsroot/mozilla/browser/app/Makefile.in,v <-- Makefile.in new revision: 1.153; previous revision: 1.152 done RCS file: /cvsroot/mozilla/browser/app/blocklist.xml,v done Checking in browser/app/blocklist.xml; /cvsroot/mozilla/browser/app/blocklist.xml,v <-- blocklist.xml initial revision: 1.1 done Checking in browser/installer/unix/packages-static; /cvsroot/mozilla/browser/installer/unix/packages-static,v <-- packages-static new revision: 1.158; previous revision: 1.157 done Checking in browser/installer/windows/packages-static; /cvsroot/mozilla/browser/installer/windows/packages-static,v <-- packages-static new revision: 1.160; previous revision: 1.159 done
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 beta5
Attachment #310310 - Attachment is obsolete: true
hi dave, how can this be easily tested?
(In reply to comment #25) > hi dave, how can this be easily tested? On a clean machine (no existing firefox profile), run firefox 3 and try to install one of the blocklisted addons (IDM and FDM right now). Equally on a clean machine install IDM before starting firefox then run it and you should still see the addon blocklisted.
This caused SeaMonkey Linux tinderbox to go orange (tUnit failures).
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: Firefox 3 beta5 → Firefox 3
This should fix the SeaMonkey test
Comment on attachment 310705 [details] [diff] [review] fix SeaMonkey TUnit take 2 (Checked in) >Index: toolkit/mozapps/extensions/src/nsBlocklistService.js >+ LOG("Blocklist::_loadBlocklist: no XML File not found"); Fix that typo :)
Attachment #310705 - Flags: review? → review+
Comment on attachment 310705 [details] [diff] [review] fix SeaMonkey TUnit take 2 (Checked in) Fixed with nit addressed Checking in toolkit/mozapps/extensions/src/nsBlocklistService.js; /cvsroot/mozilla/toolkit/mozapps/extensions/src/nsBlocklistService.js,v <-- nsBlocklistService.js new revision: 1.11; previous revision: 1.10 done
Attachment #310705 - Attachment description: fix SeaMonkey TUnit take 2 → fix SeaMonkey TUnit take 2 (Checked in)
Status: REOPENED → RESOLVED
Closed: 13 years ago → 13 years ago
Resolution: --- → FIXED
Priority: P2 → P1
Target Milestone: Firefox 3 → Firefox 3 beta5
Verified fix on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre) Gecko/2008032405 Minefield/3.0b5pre Tested on FDM 1.0 and IDM 3.3, and both are disabled from Addons Manager.
Status: RESOLVED → VERIFIED
Need to include a few litmus test cases. Testcases should reflect comment #26, and also include uninstalling extensions that are blocklisted from the OS.
Added litmus tests: https://litmus.mozilla.org/show_test.cgi?searchType=by_id&id=5218 https://litmus.mozilla.org/show_test.cgi?searchType=by_id&id=5219
Flags: in-litmus? → in-litmus+
You need to log in before you can comment on or make changes to this bug.