1480082 - Remove broadcasters from the identity panel

Status: RESOLVED FIXED

(Firefox :: Site Identity, enhancement, P1)

Description

[:Paolo Amadini]

Broadcasters can be removed easily from the identity panel.

Comment 1

[:Paolo Amadini]

Attachment: Bug 1480082 - Remove broadcasters from the identity panel.

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

Comment 2

[Johann Hofmann [:johannh]]

Comment on attachment 8996693 [details]
Bug 1480082 - Remove broadcasters from the identity panel.

https://reviewboard.mozilla.org/r/260772/#review267864

Thanks!

::: browser/base/content/browser-siteIdentity.js:629
(Diff revision 1)
>        });
>      }
>  
>      // Update "Learn More" for Mixed Content Blocking and Insecure Login Forms.
>      let baseURL = Services.urlFormatter.formatURLPref("app.support.baseURL");
> -    this._identityPopupMixedContentLearnMore
> +    this._identityPopupMixedContentLearnMore.forEach(

forEach is a tiny bit slower than for..of for small arrays (bug 1355874) and by using for..of you can avoid doing the array destructuring above, so I'd recommend using that instead.

Comment 3

[:Paolo Amadini]

(In reply to Johann Hofmann [:johannh] - slow to respond, digging out of PTO backlog from comment #2)
> forEach is a tiny bit slower than for..of for small arrays (bug 1355874) and
> by using for..of you can avoid doing the array destructuring above, so I'd
> recommend using that instead.

The comments in bug 1355874 seem to indicate the results are inconclusive, and in fact a micro-benchmark I did here shows quite the opposite, which is a reminder on the perils of micro-benchmarks. Either one could in fact be wrong.

We're talking about microseconds, but here is what I've got from fastest to slowest, on five nodes:

  let nodeList = querySelectorAll();       
  let array = [...nodeList];

  1. array.forEach(e => {})
  2. for (let e of array) {}
  3. nodeList.forEach(e => {})
  4. for (let e of nodeList) {}

So, the current solution seems to be the fastest. This is also absolutely irrelevant given all the other things the surrounding code does :-)

I'm actually fine with any code style here, but the array destructuring with forEach is also used in two other cases, so I kept the existing style. We should probably update all of them if we want to switch.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/29d7c0312590
Remove broadcasters from the identity panel. r=johannh

Comment 5

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/29d7c0312590