Opened on 03/31/2016 at 12:11:56 PM

Closed on 03/31/2016 at 05:33:45 PM

#3880 closed change (fixed)

Improve behavior of Safari content blocker option on the new options page

Reported by:	sebastian	Assignee:	sebastian
Priority:	P3	Milestone:
Module:	User-Interface	Keywords:
Cc:	greiner, kzar	Blocked By:
Blocking:		Platform:	Unknown / Cross platform
Ready:	yes	Confidential:	no
Tester:	Unknown	Verified working:	no
Review URL(s):	https://codereview.adblockplus.org/29339192

Description

Background

With #2374 the Tweaks section got added to the new options page, including a new option to switch between Content Blockers and traditional event based blocking on Safari. However, meanwhile, while implementing this feature (#3687), the behavior of that option, as implemented for the old options page, slightly changed.

It turned out that disabling Content Blockers requires a browser restart. That needs to be indicated by the options page. Also we are going to use the new Content Blocker mechanism regardless of the preference if the old API isn't available. So the option should only be shown if both mechanisms are available.

What to change

Hide the option to switch between Content Blockers and legacy blocking if only the former is available.
Indicate that restarting Safari is required, next to the option, when it got disabled. However, if it get's enabled again, stop asking to restart Safari (it's not necessary then anymore).

Change History (4)

comment:1 Changed on 03/31/2016 at 12:13:01 PM by sebastian

Review URL(s) modified (diff)
Status changed from new to reviewing

comment:2 Changed on 03/31/2016 at 12:45:31 PM by greiner

Priority changed from Unknown to P3
Ready set

comment:3 Changed on 03/31/2016 at 05:33:13 PM by abpbot

A commit referencing this issue has landed:
https://hg.adblockplus.org/adblockplusui/rev/113f363b61e4

comment:4 Changed on 03/31/2016 at 05:33:45 PM by sebastian

Resolution set to fixed
Status changed from reviewing to closed

Summary:

Description:

You may use WikiFormatting here.

Type:

Priority:

Milestone:

Module:

Keywords:

Cc:

Blocked By:

Blocking:

Platform:

Ready:

Confidential:

Tester:

Verified working:

Review URL(s):

Context Navigation

#3880 closed change (fixed)

Improve behavior of Safari content blocker option on the new options page

Description

Background

What to change

Attachments (0)

Change History (4)

comment:1 Changed on 03/31/2016 at 12:13:01 PM by sebastian

comment:2 Changed on 03/31/2016 at 12:45:31 PM by greiner

comment:3 Changed on 03/31/2016 at 05:33:13 PM by abpbot

comment:4 Changed on 03/31/2016 at 05:33:45 PM by sebastian

Add Comment

Modify Ticket

Changed by kzar

Download in other formats:

Summary:
Description:	You may use WikiFormatting here. === Background === With #2374 the Tweaks section got added to the new options page, including a new option to switch between Content Blockers and traditional event based blocking on Safari. However, meanwhile, while implementing this feature (#3687), the behavior of that option, as implemented for the old options page, slightly changed. It turned out that disabling Content Blockers requires a browser restart. That needs to be indicated by the options page. Also we are going to use the new Content Blocker mechanism regardless of the preference if the old API isn't available. So the option should only be shown if both mechanisms are available. === What to change === * Hide the option to switch between Content Blockers and legacy blocking if only the former is available. * Indicate that restarting Safari is required, next to the option, when it got disabled. However, if it get's enabled again, stop asking to restart Safari (it's not necessary then anymore).
Type:		Priority:
Milestone:		Module:
Keywords:		Cc:
Blocked By:		Blocking:
Platform:		Ready:
Confidential:		Tester:
Verified working:
Review URL(s):	https://codereview.adblockplus.org/29339192