Closed Bug 1480082 Opened 6 years ago Closed 6 years ago

Remove broadcasters from the identity panel

Categories

(Firefox :: Site Identity, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 63
Tracking Status
firefox63 --- fixed

People

(Reporter: Paolo, Assigned: Paolo)

References

Details

Attachments

(1 file)

Broadcasters can be removed easily from the identity panel.
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.
Attachment #8996693 - Flags: review?(jhofmann) → review+
(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.
Pushed by paolo.mozmail@amadzone.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/29d7c0312590
Remove broadcasters from the identity panel. r=johannh
https://hg.mozilla.org/mozilla-central/rev/29d7c0312590
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: