-
Notifications
You must be signed in to change notification settings - Fork 17
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
Run test suite on browsers #19
Comments
That would be great! Can confirm that there is a regex error for this plugin in safari but not other browsers.
|
What version of Safari and what version of Mac OS? |
Safari Version 15.4 (17613.1.17.1.13) |
@ajb413 Can you get us as stack trace of the error? |
Breaks on line 1172 of a fresh common js build of Highlight.js v11.5.0 (git: 6fc781508b). I added a log line so you can see the bad input of
Edit: Looks like that comes from this line here: https://github.com/highlightjs/highlightjs-solidity/blob/master/src/common.js#L27 Edit: Seeing same error on v10.7.3 and v10.4.1 |
That's trying to put together a regex with negative look-behinds... but the library has a check for that, so I'm not sure what is going on there. |
I had some time to dig on this. I got my minified version (@2.0.5/dist/solidity.min.js) from jsdelivr.com. It looks like there was a build issue. You can see that file here. The negative look behind check is simply omitted from the minified build. You can see it is the first function at the top, after the If you change the try/catch in the minified file to this: function e(){try{new RegExp('(?<!.)');return!0
}catch(e){return!1}} then that check actually happens, and that Safari error goes away, and the package works properly |
Perhaps minified build needs different minifier arguments to not compile out the check? |
We're currently running the test suite in Node.js only. We should also run it in the browser.
The text was updated successfully, but these errors were encountered: