Skip to content

Fix bool, number culture, and string escaping in ExecuteScriptAsync with parameters #1731

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

Merged
merged 1 commit into from
Jul 5, 2016

Conversation

chylex
Copy link
Contributor

@chylex chylex commented Jul 2, 2016

Tested by running:

ExecuteScriptAsync("window.runtest", "hello", "quote'abc", -3000000, 5.5, 8F, 7.66D, true, false, null);

window.runtest prints out all arguments and their type.

Old output on most european systems, note booleans and decimal numbers:

hello | string
(syntax error due to unescaped quote)
-3000000 | number
5 | number
5 | number
8 | number
7 | number
66 | number
True | string
False | string
null | object

New output:

hello | string
quote'abc | string
-3000000 | number
5.5 | number
8 | number
7.66 | number
true | boolean
false | boolean
null | object

Closes #1728.

{
stringBuilder.Append("'");
}
stringBuilder.Append("'").Append(args[i].ToString().Replace("'","\\'")).Append("'");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This in my opinion is getting a little hard to read, please reformat for readability 👍

@chylex
Copy link
Contributor Author

chylex commented Jul 3, 2016

Updated.

@amaitland
Copy link
Member

Updated.

Thanks 👍 Hopefully find time to have a look during the coming week.

@amaitland amaitland added this to the 51.0.0 milestone Jul 3, 2016
@amaitland amaitland merged commit 2532c80 into cefsharp:master Jul 5, 2016
amaitland pushed a commit that referenced this pull request Jul 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants