-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Allow specifying the javascript version in the debug html window #1719
Allow specifying the javascript version in the debug html window #1719
Comments
@simonzack This is possible to do. However, in case the user uses multiple browsers, this might go wrong. If, in any case Chrome is used to run besides Firefox 4 (which supports 1.8 now), the tag will make the tests all fail. Even more, Karma will throw an Error. I was able to make the configuration for you and was wanting to add a PR for it, but I stumbled upon the error part of Karma when trying to add the tag when running Chrome. Maybe @dignifiedquire has an idea how to make this fail-safe? |
Not sure (I never had to use this in a real world example before) |
I have seen that, but it is not exactly what I'm looking for. The case would be that Firefox 4 supports 1.8, but Chrome (even latest version) does not. This makes it that in the karma.js-file, when the Firefox 4 browser is run, it should add the tag of ";version=1.8", otherwise not. I'd think that there should be some check which browser is being run at that moment and perhaps the version of it. This might be a solution to it, but takes the effort to update this whitelist of browsers to be updated everytime a new update comes out... EDIT: It also seems that Firefox is the only browser that supports the ";version=..." tag. So, would there be a way to check which browser actually is being run at the moment? |
@dignifiedquire: Do you know a possible way to capture the current browser being run? That would solve the issue mainly. |
@Moumi capture it where? |
@dignifiedquire: At https://github.com/karma-runner/karma/blob/master/lib/middleware/karma.js just before the return statement: |
@dignifiedquire ping |
I don't think that will easily work as the file is simply served to every browser the same atm. You could check the express docs how to do browser detection in a single request |
@dignifiedquire I managed to get the full functionality working. I wanted to create a PR, but I wanted to write a test or 2 for it too. However, it seems very difficult to test it. The check to add the tag depends on:
Well, how can you force during a test that the user-agent should say "Firefox"? If you could help me with figuring this out, that'd be great and this issue would be fixed (I hope). I am just learning about Sinon and it isn't that clear to me either. |
@Moumi your best bet is to use an e2e test (see |
I managed to create a fix and two e2e tests for it. I will provide a PR this evening for it. |
Add the configuration to add a javascript version tag to the loaded scripts. Only applied when the Firefox browser is run. Closes karma-runner#1719
Add the configuration to add a javascript version tag to the loaded scripts. Only applied when the Firefox browser is run. Closes karma-runner#1719
Add the configuration to add a javascript version tag to the loaded scripts. Only applied when the Firefox browser is run. Closes karma-runner#1719.
Add the configuration to add a javascript version tag to the loaded scripts. Only applied when the Firefox browser is run. Closes karma-runner#1719.
Add the configuration to add a javascript version tag to the loaded scripts. Only applied when the Firefox browser is run. Closes karma-runner#1719.
Add the configuration to add a javascript version tag to the loaded scripts. Only applied when the Firefox browser is run. Closes karma-runner#1719.
I would like to test javascript 1.8 as this is what I would like my target to run on (greasemonkey on firefox), but currently
SCRIPT_TYPE
is hard-codedwithout
application/javascript;version=1.8
firefox runs ES5 only, so can there be some option to put this in the config?The text was updated successfully, but these errors were encountered: