Skip to content

/ servo

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

Jump to bottom

JavaScript toLowerCase fails on non-ASCII strings #1068

Closed
kmcallister opened this issue Oct 15, 2013 · 5 comments
Closed

JavaScript toLowerCase fails on non-ASCII strings #1068

kmcallister opened this issue Oct 15, 2013 · 5 comments
Assignees
@kmcallister
Labels
A-content/uncategorized I-crash

Comments

@kmcallister
Copy link
Contributor

@kmcallister kmcallister commented Oct 15, 2013 

<html><body><script>
alert('á'.toLowerCase());
</script></body></html>

produces

task <unnamed> failed at 'assertion failed: is_utf8(v)', /home/keegan/proj/servo/servo/src/compiler/rust/src/libstd/str.rs:979

c.f. #282, general ticket on string representation
@jdm
Copy link
Member

@jdm jdm commented Oct 15, 2013

It's the alert that actually causes us problems, isn't it?
@kmcallister
Copy link
Contributor Author

@kmcallister kmcallister commented Oct 15, 2013

Nope, for me alert('á'); produces the proper output. (But I didn't think to test that, so thanks for suggesting it.)
@kmcallister
Copy link
Contributor Author

@kmcallister kmcallister commented Oct 15, 2013

But evaluating the expression 'á'.toLowerCase(); succeeds, so I guess it's producing invalid UTF-8 (or rather UCS-2 that we convert to invalid UTF-8?) and then we fail trying to print it.
@jdm
Copy link
Member

@jdm jdm commented Oct 15, 2013

Right, I bet we fail under jsval_to_str which is called from the argument conversion glue for alert.
@ghost ghost assigned kmcallister Oct 15, 2013
@kmcallister
Copy link
Contributor Author

@kmcallister kmcallister commented Oct 16, 2013

Also we're calling JS_EvaluateScript rather than JS_EvaluateUCScript so non-ASCII literals in scripts won't work anyway.
ChrisParis pushed a commit to ChrisParis/servo that referenced this issue Sep 7, 2014
@plehegar
Merge pull request servo#1068 from phoglund/master
11ff1a0 
Bringing GetUserMedia tests more up to date.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kmcallister kmcallister

Labels
A-content/uncategorized I-crash
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
@jdm @kmcallister
You can’t perform that action at this time.