Update external dependencies

[tvdijen]

Needs testing, but the changelogs give no reason to expect any issues

[khlr]

Hello Tim!

Thanks for helping to keep up with the dependencies!

I think there's no problem in replacing pako 2.0.4 with 2.1.0.
Looking at the diff there seems to be literally a single change in pako_inflate.min.js: the version number 😁
This is hardly surprising surprising (if I don't overlook something) since pako's changelog is quite easy to grasp.

But something seems to be wrong with highlight.js:

main:

PR:

Could you please have a look at this issue?

I used https://stubidp.sustainsys.com for testing.

[tvdijen]

Hmm, I just realized I've seen this same issue elsewhere ... Seems they have changed some defaults between 11.3 and 11.8 or something

[khlr]

What's a bit strange: I can't reproduce the unwanted behaviour out of SAML-tracer. Have a look at this fiddle: https://jsfiddle.net/fkwb3p8e/1/

There's a token string assigned to a div's textContent-property. It's done pretty much the same way as SAML-tracer does it.
But in the fiddle the whole token gets highlighted correctly...

Any ideas?

[tvdijen]

The only possible explanation I found is in the highlight.js docs:

We strongly recommend <pre><code> wrapping for code blocks. It's quite semantic and "just works" out of the box with zero fiddling. It is possible to use other HTML elements (or combos), but you may need to pay special attention to preserving linebreaks.

I think maybe that's that we're seeing in your screenshot..

[khlr]

I saw that recommendation, too. But I'm not sure it "applies" to us. SAML-tracer has always represented the highlighting token this way and never used a <code> tag
Sure, the argument "we have always done it this way" is not exactly the best now 😁 But I just wonder why it should cause problems right now.

And the hint only talks about line breaks... 🤔

[tvdijen]

I guess the highlight package has changed over time too...
We have the same problem in SimpleSAMLphp where we use highlight.js to highlight metadata XML.. The entire color scheme has changed during the upgrade, but nothing in the changelog that could possibly explain this.

[khlr]

I took the liberty to update hljs to version 11.9.0. Whatever was changed with this version... The highlighting works now in any case again as desired...

@tvdijen: Would you please have a look, too?

[khlr]

Hmmm... I just took a look at the diff in our repo from version 11.8.0 to 11.9.0:
Search for the string registerLanguage(".
Did you just include the JavaScript-package? When I downloaded hljs today, I included HTTP and XML. Those two packages are required for the highlighting in SAML-tracer. Maybe this was just the cause for the incorrect highlighting, as hljs tried to highlight the token as JavaScript?

[tvdijen]

I just googled to get an updated version of the highlight.min.js file.. I was not aware of any additional packages. How did you update to 11.9?

[tvdijen]

Ah, I think I understand where I went wrong :') Good to know this, because I was going to create a workflow for auto-updating our dependencies, but this means I have to build a custom highlight.min.js package

[khlr]

Ah ok :-)

Here you can define the included packages. That's where I picked XML and HTTP. I then downloaded the ZIP file and took the highlight.min.js file.

[khlr]

I was going to create a workflow for auto-updating our dependencies

Very interesting! How are you going to achieve this? Using GitHub actions or something?

[khlr]

Regarding the PR: From my perspective everything seems to look ok and you can merge the PR 😊

[tvdijen]

Let me rephrase that:

My goal was to migrate the build-script to actions:
main...feature/autobuild

Now, this will work because the NPM-package for Highlight.js includes all 190-something languages. I could slightly improve this by building my own highlight.min.js file based on highlight.js/core + the two language packs, but since this is an extension and not a package transfered over the line, the added value would be minimal.

[khlr]

That sounds really promising, Tim!

However I would suggest that we should discuss this in another issue resp. alongside another PR's discussion, do you agree?

Do you agree then in merging this PR here?