1562575 - [Protections Panel] Move content blocking section into protections panel and update tests

Status: RESOLVED FIXED

(Firefox :: Site Identity, task, P1)

Comment 1

[(inactive) Nihanth Subramanya [:nhnt11]]

I was looking at our content blocking code and thinking about this today, and I'm now reconsidering whether we should do this already. I left the content blocking parts of the new panel for 70 because it made more sense not to duplicate all that stuff.

What I'm wondering specific to this bug is, it makes sense when implementing the new content blocking UI to just modify browser-contentblocking.js to use the new elements we add in. Does it make sense to remove all the content blocking code first? This might result in removal of code that's needed later.

Is there a benefit of removing it first and adding it back in, to prevent duplication? I want to say "no", but maybe there is a case for it...

I'll think about this more; let's discuss this in triage/standup and come up with a solid strategy.

Comment 2

[(inactive) Nihanth Subramanya [:nhnt11]]

Tim and I talked about this today and decided it might be a good idea to first move the content blocking section "as is" into the new panel, and then iterate on it to bring it to parity with the new spec. So I'm renaming this bug accordingly.

Comment 3

[Tim Huang[:timhuang]]

Unassigned myself for now, because Nihanth might want to do this for blocking and non-blocking works.

Comment 4

[Johann Hofmann [:johannh]]

(In reply to Nihanth Subramanya [:nhnt11] from comment #2)

Tim and I talked about this today and decided it might be a good idea to first move the content blocking section "as is" into the new panel, and then iterate on it to bring it to parity with the new spec. So I'm renaming this bug accordingly.

I agree this sounds like the best idea.

Comment 5

[Johann Hofmann [:johannh]]

Attachment: Bug 1562575 - Part 1 - Remove content blocking telemetry events from identity popup telemetry. r=ewright,timhuang

This lives in the protections popup now and will get a new probe.

Since I'm only removing data collection I don't think a data steward is required to sign off.

I'm removing recordClick calls in identityPanel.inc.xul in a later patch.

Depends on D37384

Comment 6

[Johann Hofmann [:johannh]]

Attachment: Bug 1562575 - Part 2 - Move content blocking section into the protections panel. r=ewright,timhuang

This is a pretty big change on paper, but most of the lines are copy-pasted, with some small
adjustments to get the content blocking code comfortable in browser-siteProtections.js.

Depends on D37555

Comment 7

[Johann Hofmann [:johannh]]

Attachment: Bug 1562575 - Part 3 - Test updates for content blocking section in browser-siteProtections.js. r=ewright,timhuang

Depends on D37556

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/de006229ba8f
Part 1 - Remove content blocking telemetry events from identity popup telemetry. r=ewright
https://hg.mozilla.org/integration/autoland/rev/c9b01b908020
Part 2 - Move content blocking section into the protections panel. r=ewright
https://hg.mozilla.org/integration/autoland/rev/fce18a9477b0
Part 3 - Test updates for content blocking section in browser-siteProtections.js. r=ewright

Comment 9

[Dorel Luca [:dluca]]

Backed out 3 changesets (bug 1562575) for Mochitest failures in browser/tools/mozscreenshots/controlCenter/browser_controlCenter.js. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=256207492&repo=autoland&lineNumber=1835

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=fce18a9477b0edc750db2c289c93c1ba10537483

Backout:
https://hg.mozilla.org/integration/autoland/rev/f82b838b02ef869a1a084810f9fa3f38c7fd13da

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/24bfd549a6f9
Part 1 - Remove content blocking telemetry events from identity popup telemetry. r=ewright
https://hg.mozilla.org/integration/autoland/rev/3eaf35e40936
Part 2 - Move content blocking section into the protections panel. r=ewright
https://hg.mozilla.org/integration/autoland/rev/02f290e8b495
Part 3 - Test updates for content blocking section in browser-siteProtections.js. r=ewright

Comment 11

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/24bfd549a6f9
https://hg.mozilla.org/mozilla-central/rev/3eaf35e40936
https://hg.mozilla.org/mozilla-central/rev/02f290e8b495

Comment 12

[Johann Hofmann [:johannh]]

Sorry, I don't understand the needinfo here :)

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Sorry, that belonged into bug 1578265.