- Issue created by @kristiaanvandeneynde
- Merge request !117Move both local JS files to head using defer → (Merged) created by kristiaanvandeneynde
- Status changed to Needs review
6 months ago 12:48pm 19 June 2024 - 🇩🇪Germany Anybody Porta Westfalica
Thanks @kristiaanvandeneynde!
head: true
is definitely wrong!For the other changes: I guess this makes a lot of sense. I'm not yet sure if this can break anything in the COOKiES functionality or cause race-conditions. But thinking about this for a while I don't think so, because the script-blocking is server side.
Any other potential risks you can see?
- 🇩🇪Germany Anybody Porta Westfalica
As the preloader already had the defer attribute, I agree it might make sense to have the same for the conf.
After testing this, we should first keep it in dev and test it for some time. The most risk I can see is, that other scripts and especially analytics scripts execution might change? (But I actually don't think so, just to write down my thoughts and potential side-effects).
Another thing we need to ensure is, that the both libraries are loaded in the correct order.
@kristiaanvandeneynde I'd very much like to have your feedback and also ping @Grevil and @JFeltkamp for feedback here, before we commit this.
- Assigned to Grevil
- Issue was unassigned.
- Status changed to RTBC
6 months ago 7:30am 28 June 2024 - 🇩🇪Germany Grevil
LGTM! Just tested a bit around with it. Everything is still working as intended!
- Status changed to Fixed
6 months ago 7:34am 28 June 2024 - 🇩🇪Germany Grevil
Merged!
Didn't change the MR title, so it doesn't show up in this issue.
- 🇧🇪Belgium kristiaanvandeneynde Antwerp, Belgium
Hey, sorry for being unresponsive. I'm currently at Dev Days and not keeping an eye on my issue queue as much. Thanks for committing this, though!
- 🇩🇪Germany Anybody Porta Westfalica
No worries @kristiaanvandeneynde! :) But please feel free to test the latest dev and reply here.
Automatically closed - issue fixed for 2 weeks with no activity.