Re: [PATCH] Add the capability to import and export settings

[Yuchen Pei]

Hi Mónica,
On Tue 2024-01-30 15:04:22 -0600, Mónica Gómez wrote:

> Signed-off-by: Mónica Gómez <eunbyeol64@naver.com>
> ---
>  NEWS                                          |  4 +
>  html/preferences_panel/pref.js                | 85 ++++++++++++++++++-
>  html/preferences_panel/preferences_panel.html | 10 ++-
>  html/preferences_panel/prefs.css              | 16 ++++
>  manifest.json                                 |  2 +-
>  test/spec/LibreJSSpec.js                      | 43 ++++++++++
>  6 files changed, 154 insertions(+), 6 deletions(-)

Thanks a lot for the updated patch. The next two weeks I have less
time to sit down at my computer for some uninterrupted time outside of
work, so please bear with me with the actual review. Meanwhile I took
a look at the patch and a few questions come to my mind that I would
like to figure out (but if anyone has answers feel free to reply):

- how will this feature work with or be developed into incorprating the
  other idea of having librejs read the config file for black and white
  lists on startup?

- in your patch it reads and writes a conf file by manually parsing -
  could it be better or necessary to use an existing library that
  handles such files, or would it be easy to do the manual parsing now
  and switch to a library when needed?

- external scripts are ok in the conf file as they are shown as "normal"
  http(s) urls with some globbing patterns, like
  https://forum.members.fsf.org/*
  but inline scripts are represented in a funny way, like so

--8<---------------cut here---------------start------------->8---
  inline://www.fsf.org#(<SCRIPT>) // @license 
mag…8855c4df2063479cf018bb81d75c6434c99b104fa0fcc40a45920792711650ec
--8<---------------cut here---------------end--------------->8---

  The above is what represents the inline scripts on the fsf homepage:

--8<---------------cut here---------------start------------->8---
// @license 
magnet:?xt=urn:btih:1f739d935676111cfff4b4693e3816e664797050&dn=gpl-3.0.txt 
GPL-3.0-or-later
  var _paq = window._paq = window._paq || [];
  /* tracker methods like "setCustomDimension" should be called before 
"trackPageView" */
  _paq.push(['trackPageView']);
  _paq.push(['enableLinkTracking']);
  (function() {
    var u="https://piwik.fsf.org/";;
    _paq.push(['setTrackerUrl', u+'matomo.php']);
    _paq.push(['setSiteId', '5']);
    var d=document, g=d.createElement('script'), 
s=d.getElementsByTagName('script')[0];
    g.type='text/javascript'; g.async=true; g.src=u+'matomo.js'; 
s.parentNode.insertBefore(g,s);
  })();
  // @license-end
--8<---------------cut here---------------end--------------->8---

  will such lossy encoding reliably enough?

- what do the tests cover? do they cover for example inline url cases?

> [... 240 lines elided]

Best,
Yuchen

--
Dr Yuchen Pei | https://ypei.org | Timezone: UTC+11
PGP Key: 47F9 D050 1E11 8879 9040  4941 2126 7E93 EF86 DFD0
https://ypei.org/assets/ypei-pubkey.txt