Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#1489 closed change (fixed)

Enable pre-configurable properties in Firefox

Reported by: sebastian Assignee: fhd
Priority: P3 Milestone:
Module: Automation Keywords: growth, large-scale-deployments
Cc: mapx, simona, sebastian, trev Blocked By:
Blocking: #542, #2347 Platform: Firefox
Ready: yes Confidential: no
Tester: Verified working: no
Review URL(s):

http://codereview.adblockplus.org/6647895159734272/

Description (last modified by fhd)

Background

In order to help administrators to deploy Adblock Plus in managed networks, we introduce a mechanism to enable pre-configurable properties.

What to change

On Firefox, we can use prefs for this, those can be set externally. However, Adblock Plus is currently overwriting default prefs - we shouldn't do that for preconfigured prefs.

Change History (26)

comment:1 Changed 5 years ago by sebastian

  • Blocking 206 added

comment:2 Changed 5 years ago by mapx

  • Description modified (diff)

comment:3 Changed 5 years ago by mapx

  • Cc mapx added

comment:4 Changed 5 years ago by sebastian

  • Description modified (diff)

comment:5 Changed 5 years ago by sebastian

  • Cc sebastian removed

comment:6 Changed 5 years ago by simona

  • Keywords growth added

comment:7 Changed 5 years ago by simona

  • Cc simona added

comment:8 Changed 5 years ago by simona

  • Keywords large-scale-deployments added

comment:9 Changed 4 years ago by fhd

  • Blocking 206 removed

comment:10 Changed 4 years ago by fhd

Firefox already has a mechanism for pre-configuring prefs.

It's already possible to preconfigure ABP to some extent this way. However, users can't really change prefs set this way - those will be overwritten on each startup.

Pre-configuring the default prefs seems like a better option. However, currently ABP is overwriting the default prefs - for this to work we need to leave default prefs that have already been set alone.

I've put up a patch for that. However, so far we don't agree on this being the best approach.

Last edited 4 years ago by fhd (previous) (diff)

comment:11 Changed 4 years ago by sebastian

  • Blocked By 1487 removed

comment:12 Changed 4 years ago by sebastian

  • Cc sebastian added
  • Ready unset

comment:13 Changed 4 years ago by fhd

  • Component changed from Adblock-Plus-for-Firefox to Build-and-Release-Tools
  • Description modified (diff)
  • Review URL(s) modified (diff)

By now it seems pretty clear to me that this is actually a buildtools issue, changing the component. We're dealing with default prefs in buildtools, we'll have to deal with preconfigured prefs there as well.

Also properly linking the patch I put up for this. Still, not set to Ready - Sebastian should do that if he's fine with the issue.

Last edited 4 years ago by fhd (previous) (diff)

comment:14 Changed 4 years ago by fhd

  • Owner set to fhd

comment:15 follow-up: Changed 4 years ago by fhd

I suppose the major alternative to the preconfigured_defaults pref would be to have preconfigured defaults on a separate branch, e.g. extensions.adblockplus.preconfigured, shouldn't be that much more complicated how I see it.

Which approach would you prefer, Wladimir?

comment:16 Changed 4 years ago by fhd

  • Cc trev added

Wladimir: Oh, asked you something when you weren't even in CC :) Please see above.

comment:17 in reply to: ↑ 15 ; follow-up: Changed 4 years ago by trev

Replying to fhd:

I suppose the major alternative to the preconfigured_defaults pref would be to have preconfigured defaults on a separate branch, e.g. extensions.adblockplus.preconfigured, shouldn't be that much more complicated how I see it.

Sounds good to me, we would then also be able to apply the same whitelist approach we have in Chrome right now. So only extensions.adblockplus.preconfigured.suppress_first_run_page would be considered but not extensions.adblockplus.preconfigured.enabled for example. Given that we import the defaults ourselves, it wouldn't be a large change.

Note that I would change the prefs.js file format (and rename the file) while at it - JSON listing the default preferences (without branch name) and a whitelist for preconfigured preferences sounds like a much better idea.

comment:18 in reply to: ↑ 17 Changed 4 years ago by fhd

Alright, will put up a new patch with this approach.

Replying to trev:

Note that I would change the prefs.js file format (and rename the file) while at it - JSON listing the default preferences (without branch name) and a whitelist for preconfigured preferences sounds like a much better idea.

Sounds good, but I'd prefer to keep this patch minimal if possible. I've prepared one that'll introduce a third parameter to pref() that marks preconfigurable prefs.

Last edited 4 years ago by fhd (previous) (diff)

comment:19 Changed 4 years ago by fhd

  • Ready set
  • Status changed from new to reviewing

comment:20 Changed 4 years ago by fhd

  • Blocking 2347 added

comment:21 Changed 4 years ago by fhd

This has landed. Note that there's a follow-up for making the suppress_first_run_page pref preconfigurable: #2439

comment:22 Changed 4 years ago by fhd

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:23 Changed 4 years ago by fhd

  • Blocking 2347 removed

comment:24 Changed 4 years ago by fhd

  • Blocking 2347 added

comment:26 Changed 4 years ago by philll

  • Platform changed from Firefox/Firefox Mobile to Firefox

Made Firefox and Firefox mobile available as seperate platforms.

Note: See TracTickets for help on using tickets.