Closed Bug 336290 Opened 14 years ago Closed 3 years ago
remove --enable-safe-browsing flag once safebrowsing lands
MozReview Request: Bug 336290 - remove --enable-safe-browsing from configure since its used everywhere. r?gps,gcp
58 bytes, text/x-review-board-request
After safebrowsing is fully integrated, remove the configure flag and turn on by default in bon echo and trunk. This is a follow up on to bug 334575 where the configure flag was added.
Reed, this was 3 years back. I'm guessing it's been resolved?
safebrowsing is enabled by default, but the configure flag still exists... we should just remove it.
The current state is: safe browsing on: Firefox safe browsing off: Fennec Native Fennec XUL B2G While I believe it is our goals and intentions to bring safe browsing to these other platforms, it looks like this configuration flag is still bringing us positive value. source: https://mxr.mozilla.org/mozilla-central/search?string=MOZ_SAFE_BROWSING&find=confvars.sh&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central
Review commit: https://reviewboard.mozilla.org/r/44015/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/44015/
Assignee: nobody → jaws
Status: NEW → ASSIGNED
OS: Windows XP → All
Hardware: x86 → All
https://reviewboard.mozilla.org/r/44015/#review40799 Something isn't exactly right with this patch as XPCOMUtils.jsm and specialpowersAPI.js fail to get included. ::: old-configure.in (Diff revision 1) > -dnl Implicitly enabled by default if building with safe-browsing > -if test -n "$MOZ_SAFE_BROWSING"; then > - MOZ_URL_CLASSIFIER=1 > -fi > MOZ_ARG_ENABLE_BOOL(url-classifier, > [ --enable-url-classifier Enable url classifier module], MOZ_URL_CLASSIFIER should probably be default enabled now that MOZ_SAFE_BROWSING is enabled by default.
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #6) > MOZ_URL_CLASSIFIER should probably be default enabled now that > MOZ_SAFE_BROWSING is enabled by default. We can probably remove MOZ_URL_CLASSIFIER too since it's a requirement of both Safe Browsing and Tracking Protection.
Comment on attachment 8737562 [details] MozReview Request: Bug 336290 - remove --enable-safe-browsing from configure since its used everywhere. r?gps,gcp https://reviewboard.mozilla.org/r/44015/#review41027 It is my understanding that any Firefox feature that touches external services needs to have a flag to disable it, as downstream clients like Tor may want to disable it (or replace it with their own mechanisms). This is because pinging external services constitutes a side-channel data leak. Furthermore, I believe enabling safe browsing by default will have implications for developer builds and possibly Talos (the latter likely already has it disabled by default). IIRC, enabling safe browsing causes the browser to download the safe browsing data files shortly after startup on first run. If we have developer builds doing this, that could have unintended consequences. What I'm not sure is if this flag needs to be at the build system level or whether it is sufficient to have a preference. I'd like to think a preference is OK. After all, we prefer to have N-1 build system configurations and this patch does remove one variation.
As long as building the code doesn't cause third parties problems, then having the ability to turn features off by preference seems fine.
(In reply to Gregory Szorc [:gps] from comment #8) > What I'm not sure is if this flag needs to be at the build system level or > whether it is sufficient to have a preference. I'd like to think a > preference is OK. After all, we prefer to have N-1 build system > configurations and this patch does remove one variation. We have prefs to disable all of Safe Browsing and suppress all of the network requests. As of bug 1259288, they are fully disabled in all of our tests. These are more likely to keep working than the enabled-by-default configure flags IMHO.
Comment on attachment 8737562 [details] MozReview Request: Bug 336290 - remove --enable-safe-browsing from configure since its used everywhere. r?gps,gcp https://reviewboard.mozilla.org/r/44015/#review41317 ::: old-configure.in:2801 (Diff revision 1) > MOZ_SPELLCHECK=1 > MOZ_ANDROID_APZ= > MOZ_TOOLKIT_SEARCH=1 > MOZ_UI_LOCALE=en-US > MOZ_UNIVERSALCHARDET=1 > MOZ_URL_CLASSIFIER= So remove this as well as it's no longer optional?
Attachment #8737562 - Flags: review?(gpascutto) → review+
This was r+ed but did not land?
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/fx-team/rev/d3ef59a2f50a remove --enable-safe-browsing from configure since its used everywhere. r=gcp
You need to log in before you can comment on or make changes to this bug.