Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"invalid group specifier name" on MacOS Safari #225

Closed
romkavt opened this issue Feb 19, 2021 · 9 comments · Fixed by #228
Closed

"invalid group specifier name" on MacOS Safari #225

romkavt opened this issue Feb 19, 2021 · 9 comments · Fixed by #228
Labels
🐞 bug Something isn't working 🧭 Safari

Comments

@romkavt
Copy link

romkavt commented Feb 19, 2021

Hello!

asciidoctor-kroki.js version 0.12
On Safari 14.0.3 with MacOS 10.15.7 (Catalina) the PlantUML diagrams doesn't render with the following error:

[Error] SyntaxError: Invalid regular expression: invalid group specifier name
	(anonymous function) (asciidoctor-kroki.js:18656)

Thanks for help in advance

@ggrossetie
Copy link
Member

Safari does not support lookbehind in regular expressions (sigh): https://caniuse.com/js-regexp-lookbehind

https://github.com/Mogztter/asciidoctor-kroki/blob/05623c9ea0bbf29c11bad848ee529f8694b9f2f3/src/preprocess.js#L106

To accommodate Safari we should rewrite this regular expression to avoid using lookbehind. It might require to do additional checks.

@ggrossetie ggrossetie added 🐞 bug Something isn't working 🧭 Safari labels Feb 19, 2021
@ggrossetie
Copy link
Member

@romkavt The line number does not correspond but the error tells me that this is related to a regular expression. Do you have a more complete stacktrace ?

@secdim
Copy link

secdim commented Jul 30, 2021

@Mogztter can we get this fix for Safari merged in?

@ggrossetie
Copy link
Member

@secdim If you could confirm that #228 actually resolves this issue in Safari that would be helpful. I don't have Safari so I can't make sure that my fix is sufficient.

@romkavt
Copy link
Author

romkavt commented Jul 30, 2021

@secdim If you could confirm that #228 actually resolves this issue in Safari that would be helpful. I don't have Safari so I can't make sure that my fix is sufficient.

I merged this 'by hands' in our installation and I confirm the fix works well on Mac OS Safari.

@ggrossetie
Copy link
Member

Thanks for confirming, I will merge the pull request!

@secdim
Copy link

secdim commented Aug 8, 2021

@secdim If you could confirm that #228 actually resolves this issue in Safari that would be helpful. I don't have Safari so I can't make sure that my fix is sufficient.

Yes, I have tested it and it fixes Safari issue.

@ggrossetie
Copy link
Member

@secdim Thanks for confirming, I will release a new version today.

@ggrossetie
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working 🧭 Safari
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants