Added ZIM metadata to security dialogue box when opening ZIM for the first time

[D3V-D]

Fix for #1247
Ran npm run serve and npm run test and npm run preview
Also did npm run build followed by npm run serve and tested on the /dist page.

Tested firefox + chromium. (not tested on ie11 yet)

Pretty sure jQuery mode testing is not applicable as the security dialogue is only for serviceWorker mode (correct me if I'm wrong).

Added french and spanish translations (and English) with deepl, feel free to double check.

Please review, @Jaifroid . Thanks!

[D3V-D]

Just wanted to add, there's a lot of other ZIM metadata I think could be used well here; so I don't know if we'd want to pursue that? Like including date of creation, description, etc.

[Jaifroid]

Just wanted to add, there's a lot of other ZIM metadata I think could be used well here; so I don't know if we'd want to pursue that? Like including date of creation, description, etc.

There are indeed, but each one has a "cost", in that we have to load them up-front from the ZIM for them to be available at the time these metadata are shown. Currently, most metadata aren't needed at ZIM load time, and they load in the background in order to prioritize opening the ZIM as fast as possible for the user. So I judged that these are the most important ones (when I coded it for the PWA).

[Jaifroid]

I've tested on Edge -- all functionality appears to work as intended. In IE11 of course the security dialogue doesn't show because it's already running in jQuery, aka "Safe", mode, but the code doesn't cause any issues for the browser. So, all working well!

I'll do a code review asap.

[D3V-D]

Yep, please let me know if I did something strange when retrieving the metadata for the alert, the solution felt somewhat obtuse.

[D3V-D]

Also, did you link this PR to the corresponding issue?

[Jaifroid]

Also, did you link this PR to the corresponding issue?

Done. Normally it links automatically if you write "Fixes .....", maybe because you wrote "Fix for ..." (and it's not a very intelligent algorithm)!

[Jaifroid]

Actually the algo is more intelligent than I give it credit for:

[Jaifroid]

@D3V-D Many thanks for this PR. It works, as I mentioned, and you came up with some creative solutions. There are, however, a few things that could do with refactoring to avoid putting your code into generic functions, and also to ensure metadata are fetched at the appropriate time in a way that can be used by other app functions. All explained in comments.

Regarding translations, thanks for doing these. I'll check these properly once the code is ready.

Don't hesitate to ask if you have any queries!

[D3V-D]

@Jaifroid many thanks for the feedback! Yeah I did think of passing in HTML directly, but I wasn't sure if that was a good idea or not, but I guess it would be better for the reasons you stated.

I'll also look into the different ways of getting the metadata, and get back to you with an update

[Jaifroid]

@Jaifroid many thanks for the feedback! Yeah I did think of passing in HTML directly, but I wasn't sure if that was a good idea or not, but I guess it would be better for the reasons you stated.

I'll also look into the different ways of getting the metadata, and get back to you with an update

Thanks! I don't think it will take very long, as it's mostly moving a few things around.

[D3V-D]

Alright, everything should be in order now. Please review again (and any testing necessary to ensure it's still functional, but it all seems fine on my end). @Jaifroid

[Jaifroid]

That's much better now, many thanks! It's looking very good. Just some really minor last things (punctuation, translations, ...).

[Jaifroid]

I also just noticed this, which doesn't show on main. You are probably using textContent somewhere and it might literally interpret   as text rather than as HTML? You can probably just substitute innerHTML at the appropriate point.

[D3V-D]

Yeah, I'll take a look at all those things.

[D3V-D]

Okay, I've fixed pretty much all the things you said, just one issue.

When putting the text for the verification metadata, I avoided using innerHTML and instead used innerText to avoid (since the metadata can be "spoofed") any kind of security issues. However, this makes it hard to use the   for French. If necessary to use that instead of a normal blank space, I can try using innerHTML for the translated text then followed by innerText to insert the actual metadata.

[Jaifroid]

OK, great. I understand the security concern with innerHTML if we are inserting a string that we do not control and may be a vector for attack. We do, of course, control the metadata labels, so it should be safe to insert at least those with innerHTML. The alternative would be to sanitize the metadata themselves, potentially in the getMetadata function (or whatever it's called).

[Jaifroid]

Actually, no need to complicate the code, as you're right that we don't actually need   where a space will do and there is no risk of the colons being orphaned (no screen will ever be that narrow). So leave it as you have it now.

[Jaifroid]

All looking good now! Perhaps just change that type from Object to ZIMArchive, and it's ready.

[D3V-D]

All done! Feel free to merge whenever @Jaifroid

[Jaifroid]

Great, thanks for the PR!