1231373 - [experiment] Add a dialog to inspect or remove exceptions from TP

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect, P1)

Description

[Panos Astithas (he/him) [:past] (please ni?)]

Users may want to view/delete any (or all) existing site-level TP exceptions, which will disable TP for that site. This dialog can use a simplified version of the exceptions dialog we have for the popup blocker exceptions.

Comment 1

[:Paolo Amadini]

Attachment: MozReview Request: Bug 1231373 - Add a dialog to inspect or remove exceptions from TP.

Review commit: https://reviewboard.mozilla.org/r/29349/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/29349/

Comment 2

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 8703347 [details]
MozReview Request: Bug 1231373 - Add a dialog to inspect or remove exceptions from TP.

https://reviewboard.mozilla.org/r/29349/#review26371

Looks great!

::: browser/components/preferences/in-content/privacy.js:432
(Diff revision 1)
> +  showTrackingProtectionExceptions: function ()
> +  {
> +    var bundlePreferences = document.getElementById("bundlePreferences");
> +    var params = { allowVisible   : true,

My usual nits: opening curly and let instead of var :)

Comment 3

[:Paolo Amadini]

Attachment: Screenshot of the Exceptions dialog for Tracking Protection

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/105bbb3e0c5a

Comment 5

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/105bbb3e0c5a

Comment 6

[Paul Silaghi, QA [:pauly]]

Couple of suggestions:
1. Exceptions dialog updates live (if opened) when a site is added to the list (from control center/disable TP), but doesn't update when the TP is enabled back
2. It would be nice to have the "Save changes" button available only if there have been changes made
Let me know your thoughts and if I should file anything separately.

Comment 7

[:Paolo Amadini]

You can file these improvements as two separate bugs. We use a common dialog for all the permission types, of which Tracking Protection is only one, and these improvements apply to all of them.

Comment 8

[Paul Silaghi, QA [:pauly]]

(In reply to :Paolo Amadini from comment #7)
> these improvements apply to all of them.
Done. Feel free to set the dependencies/components properly. Thanks.

Comment 9

[:Paolo Amadini]

(In reply to Paul Silaghi, QA [:pauly] from comment #8)
> Done. Feel free to set the dependencies/components properly. Thanks.

They sound right. Thanks for filing the bugs!

Comment 10

[Paul Silaghi, QA [:pauly]]

One more question:
Should the exceptions apply for a private window too? Or should I be able to add exceptions in PB?

Comment 11

[:Paolo Amadini]

(In reply to Paul Silaghi, QA [:pauly] from comment #10)
> Should the exceptions apply for a private window too?

Permanent exceptions set in normal windows should later apply to private windows too.

> Or should I be able to add exceptions in PB?

But you shouldn't be able to set a permanent exception from a private window.

Comment 12

[Paul Silaghi, QA [:pauly]]

(In reply to :Paolo Amadini from comment #11)
> Permanent exceptions set in normal windows should later apply to private
> windows too.
This is not happening. Bug on the way...

Comment 13

[Paul Silaghi, QA [:pauly]]

Verified fixed considering the follow-up bugs, on FF 46.0a1 (2016-01-13) Win 7, Ubuntu 14.04, OS X 10.9.5.