simplesamlphp / SAML-tracer

Browser extension for examining SAML messages
https://addons.mozilla.org/nl/firefox/addon/saml-tracer/
BSD 2-Clause "Simplified" License
141 stars 39 forks source link

Autobuild release when a tag is set #87

Closed tvdijen closed 6 months ago

tvdijen commented 10 months ago

This replaces the old build.sh script and will automatically build a release when a tag is set.

tvdijen commented 10 months ago

@khlr You know how to test this add-on? An automated build was created here, but I'm unable to load it into Firefox.. I says it's broken :(

khlr commented 10 months ago

Please excuse me for not responding yet... I think it'll take me some days to review this one.

tvdijen commented 10 months ago

While testing I found an issue preventing highlight.js to load:

Uncaught ReferenceError: hljs is not defined
    highlightContent moz-extension://e2144872-b937-4f53-955e-692204049bdc/src/ui.js:209
    highlightContent moz-extension://e2144872-b937-4f53-955e-692204049bdc/src/ui.js:206
    enableSyntaxHighlighting moz-extension://e2144872-b937-4f53-955e-692204049bdc/src/ui.js:219
    <anonymous> moz-extension://e2144872-b937-4f53-955e-692204049bdc/src/ui.js:9
    EventListener.handleEvent* moz-extension://e2144872-b937-4f53-955e-692204049bdc/src/ui.js:1

I have to figure out how to properly instantiate this hljs variable.. Maybe I have to do it in ui.js instead of in the bundle.

khlr commented 9 months ago

@thijskh: Maybe you've gotten over it. But would you like to do another review of this PR?

And could you kindly have a look at this (access to the secrets)? https://github.com/simplesamlphp/SAML-tracer/pull/87#discussion_r1382381600

tvdijen commented 9 months ago

I'm still not sure how to get this working though :(

khlr commented 9 months ago

What's not working yet, Tim?

tvdijen commented 9 months ago

Ehh, everything.. https://github.com/simplesamlphp/SAML-tracer/pull/87#issuecomment-1784969480

tvdijen commented 6 months ago

Ok, so I finally got this to work (yay)! But there is also bad news, and that is that highlight.js has dropped the 'properties' and 'http' languages. They are only available if you manually build the package.

Now I've verified that manually building works and it's really not that hard, but it completely defeats the purpose of easy-to-update dependencies.. Thoughts or suggestions are welcome.

tvdijen commented 6 months ago

Everything is working now, with a manual build of highlight.js.. We have to manually update highlight.js and pako by changing the version numbers in the GA workflow file.

tvdijen commented 6 months ago

@khlr I think we have enough changes for a new release now.. I believe you know how to upload a new version to Mozilla?

khlr commented 6 months ago

Thank you for your efforts, Tim!

Now I've verified that manually building works and it's really not that hard, but it completely defeats the purpose of easy-to-update dependencies.. Thoughts or suggestions are welcome.

I didn't look into it in detail yet, but maybe prism.js could be a suitable alternative for hljs if it's easier to use for us?

Please help me to get my head around why we do we have to build pako manually?

I think we have enough changes for a new release now.

I think so, too 😊 We could release version 1.8.0 to Mozilla, but we cannot publish it to the chrome web store yet. To do so we still need a privacy policy. I wouldn't be so happy if it was only published in one store and not the other....

tvdijen commented 6 months ago

Please help me to get my head around why we do we have to build pako manually?

We don't necessarily have to, but it keeps our build-process consistent. Pako can be installed using NPM & Webpack, but that would bring in a lot of overhead that I think isn't worth it.