1444868 - Setting permissionsText in the exceptions dialog (permissions.js) can be simplified

Status: RESOLVED FIXED

(Firefox :: Settings UI, enhancement, P5)

Description

[Johann Hofmann [:johannh]]

https://searchfox.org/mozilla-central/rev/a70da6775d5341a9cca86bf1572a5e3534909153/browser/components/preferences/permissions.js#219

Instead of iterating over child nodes and deleting them, then making a new textNode and inserting it, we can just use .textContent to override the intro text directly.

Comment 1

[Trisha Gupta [:trisha]]

(In reply to Johann Hofmann [:johannh] from comment #0)
> https://searchfox.org/mozilla-central/rev/
> a70da6775d5341a9cca86bf1572a5e3534909153/browser/components/preferences/
> permissions.js#219
> 
> Instead of iterating over child nodes and deleting them, then making a new
> textNode and inserting it, we can just use .textContent to override the
> intro text directly.
Do you mean it overwrites it?

Is this what it should do somewhat?

 var permissionsText = document.getElementById("permissionsText");
 permissionsText.textContent=aParams.introText;

Can you tell me more about aParams.introText please? Where does it come from? Can't find it anywhere in Searchfox.

Comment 2

[Johann Hofmann [:johannh]]

(In reply to Trisha from comment #1)
> (In reply to Johann Hofmann [:johannh] from comment #0)
> > https://searchfox.org/mozilla-central/rev/
> > a70da6775d5341a9cca86bf1572a5e3534909153/browser/components/preferences/
> > permissions.js#219
> > 
> > Instead of iterating over child nodes and deleting them, then making a new
> > textNode and inserting it, we can just use .textContent to override the
> > intro text directly.
> Do you mean it overwrites it?

Yeah, overwrite :)

> Is this what it should do somewhat?
> 
>  var permissionsText = document.getElementById("permissionsText");
>  permissionsText.textContent=aParams.introText;

That looks reasonable to me, you could use let instead of var and be sure to run ./mach eslint :)

> Can you tell me more about aParams.introText please? Where does it come
> from? Can't find it anywhere in Searchfox.

introText is passed by the code opening this dialog to specify the description that should show at the top of the dialog.

An example is here: https://searchfox.org/mozilla-central/rev/99df6dad057b94c2ac4d8b12f7b0b3a7fdf25d04/browser/components/preferences/in-content/privacy.js#931

Which can be seen when going to about:preferences -> Privacy & Security -> Cookies & Site Data -> Exceptions

You should verify that after your change the dialog still shows this text:

https://searchfox.org/mozilla-central/rev/99df6dad057b94c2ac4d8b12f7b0b3a7fdf25d04/browser/locales/en-US/chrome/browser/preferences/preferences.properties#25

Thanks!

Comment 3

[Trisha Gupta [:trisha]]

Attachment: bug1444868.patch

I ran the test and it gave no errors. Please review. Thanks!

Comment 4

[Johann Hofmann [:johannh]]

Comment on attachment 8958529 [details] [diff] [review]
bug1444868.patch

Review of attachment 8958529 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good to me, thank you!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/93714d07f744
use .textContent to override the intro text directly in permissions.js. r=johannh

Comment 6

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/93714d07f744