Skip to content

Preparation code output script stripping is incomplete #421

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

Open
Chessax opened this issue Sep 13, 2017 · 1 comment
Open

Preparation code output script stripping is incomplete #421

Chessax opened this issue Sep 13, 2017 · 1 comment

Comments

@Chessax
Copy link
Contributor

Chessax commented Sep 13, 2017

As far as I understand according to #228 the preparation code output should be stripped of any and all <script> tags, yet it only strips the first script tag it finds. This is due to String.prototype.replace() stopping after the first match unless a RegExp with the global flag is used.

However due to #236, being unable to edit tests, I can't really give any neat live examples.

Regex definition is here:

script: '(<script[^>]*?>)([\\s\\S]*?)(</script>)'

RegEx instantiation and replacing is here:
const reScripts = new RegExp(regex.script, 'i');
stripped = page.initHTML.replace(reScripts, '');

I.e. this RegEx():

const reScripts = new RegExp(regex.script, 'i');

Should have the global (g) flag as well:

const reScripts = new RegExp(regex.script, 'ig');

These lines will also be affected (a good thing):

page.initHTML.replace(
reScripts,

And last but not least, this line should have the global flag as well, for the same reasons:

).value.replace(/@jsPerfTagToken/, () => swappedScripts.pop());

As a side note; highlight.js 9.12.0 (and apparently the version used in jsPerf) automatically highlights javascript inside HTML-code, so the special code for swapping for @jsPerfTagToken back and forth is not really required (though I guess it doesn't really hurt). This is also the reason the code "works", because highlight.js is masking the fact that only the content of the first <script> tag is explicitly highlighted.

@mathiasbynens
Copy link
Contributor

Would you be willing to submit a PR? You’ve done all the hard work already :) (Much appreciated!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants