1547718 - Remove `new Function` from preferencesBindings.js

Status: RESOLVED FIXED

(Core :: DOM: Security, enhancement, P3)

Description

[Jonas Allmann [:jallmann]]

Three uses of new Function[1][2][3] for custom event handling in preferencesBindings.js need to be removed as part of Bug 1473549.

[1] https://searchfox.org/mozilla-central/source/toolkit/content/preferencesBindings.js#238

[2] https://searchfox.org/mozilla-central/source/toolkit/content/preferencesBindings.js#350

[3] https://searchfox.org/mozilla-central/source/toolkit/content/preferencesBindings.js#408

Comment 1

[Jonas Allmann [:jallmann]]

I would like to eliminate these three new Functions, which are used to evaluate event handling code from XUL-attributes, namely:

onbeforeaccept
onsyncfrompreference
onsynctopreference

The first is easily removed and replaced by JS-eventListeners, but the other two look more complicated.
In both cases, the code invoked upon the respective events returns values other than true/false that are processed further. I don't see an immediate way to replicate this behaviour with eventListeners.
What could be a reasonable approach here?

https://searchfox.org/mozilla-central/source/toolkit/content/preferencesBindings.js#345

https://searchfox.org/mozilla-central/source/toolkit/content/preferencesBindings.js#403

Comment 2

[:Gijs (out for now; he/him)]

Defining a "manual" JS callback/observer infrastructure in preferencesBinding.js that JS on the page can add callbacks/observers to that behave how we want (ie we use returned values if not undefined, or something) seems like the most straightforward way. There's no real need to actually use custom events.

Comment 3

[Jonas Allmann [:jallmann]]

Attachment: Bug 1547718, Remove preferenceBindings.js from eval()-whitelist, r=gijs

Work in progress.
Add callback infrastructure to preferenceBindings to replace XUL-attributes.

Comment 4

[:Gijs (out for now; he/him)]

Jorg, heads-up that we're killing onsync{to,from}preference here. I expect thunderbird might care... I also don't think this will land in soft freeze, so probably early (ish?) in the 70 cycle.

Comment 5

[Jorg K (CEST = GMT+2)]

Thanks for the heads-up, forwarding ...

Comment 6

[Magnus Melin [:mkmelin]]

Filed bug 1562560 for thunderbird.

Comment 7

[Jorg K (CEST = GMT+2)]

Umm, what about comment #4. Not landing during soft freeze, which is now. I'll remove the checkin-needed again, no offence intended.

Comment 8

[Jonas Allmann [:jallmann]]

Right, sorry. I missed that, no offence taken.

Comment 9

[:Gijs (out for now; he/him)]

Jonas, this needs a rebase now (probably due to the prettier changes). Can you rebase and upload the updated version? Sorry for the churn...

Comment 10

[Jonas Allmann [:jallmann]]

I rebased the patch. Should I flag checkin-needed now?

Comment 11

[Jorg K (CEST = GMT+2)]

Sure, sorry, for the extra hassle.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d426669c4cba
Remove preferenceBindings.js from eval()-whitelist, r=Gijs

Comment 13

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/d426669c4cba