Skip to content
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

Error reporting strings are Latin1, not UTF-8. #208

Merged
merged 2 commits into from Nov 5, 2015

Conversation

@eefriedman
Copy link
Contributor

eefriedman commented Oct 29, 2015

Because the best way to convert from a 16-bit encoding to an 8-bit
encoding is obviously truncation.

Review on Reviewable

Because the best way to convert from a 16-bit encoding to an 8-bit
encoding is obviously truncation.
@michaelwu
Copy link
Contributor

michaelwu commented Oct 29, 2015

Have you tried using report->ucmessage?

@eefriedman
Copy link
Contributor Author

eefriedman commented Oct 29, 2015

Just tried it... it contains the exact same Latin1 string inflated to u16.

@michaelwu
Copy link
Contributor

michaelwu commented Oct 29, 2015

Hmm I expected it to be the other way around. For example, in http://mxr.mozilla.org/mozilla-central/source/js/src/jscntxt.cpp#650 , ucmessage is the original string before truncation to latin1.

@eefriedman
Copy link
Contributor Author

eefriedman commented Oct 29, 2015

Added commit to use ucmessage. You can test for yourself if you want.

@eefriedman eefriedman force-pushed the eefriedman:latin1-string branch 2 times, most recently from bc3d890 to 5a5e090 Oct 29, 2015
As far as I can tell, it's precisely the same string as message... but
it's possible this might lead to better error reporting in some obscure
case I didn't run into.
@eefriedman eefriedman force-pushed the eefriedman:latin1-string branch from 5a5e090 to 9c4a2ba Oct 29, 2015
@eefriedman
Copy link
Contributor Author

eefriedman commented Oct 29, 2015

Oops; not sure how I forgot it was in the standard library. Updated.

@eefriedman
Copy link
Contributor Author

eefriedman commented Nov 3, 2015

@michaelwu Ping.

@michaelwu
Copy link
Contributor

michaelwu commented Nov 4, 2015

@eefriedman
Copy link
Contributor Author

eefriedman commented Nov 5, 2015

Nothing is happening here; do I need to file a bug against saltfs or something?

@jdm
Copy link
Member

jdm commented Nov 5, 2015

@bors-servo: r=michaelwu
I had to re-synchronize homu's queue for rust-mozjs.

@bors-servo
Copy link
Contributor

bors-servo commented Nov 5, 2015

📌 Commit 9c4a2ba has been approved by michaelwu

@bors-servo
Copy link
Contributor

bors-servo commented Nov 5, 2015

Testing commit 9c4a2ba with merge bba1b42...

bors-servo added a commit that referenced this pull request Nov 5, 2015
Error reporting strings are Latin1, not UTF-8.

Because the best way to convert from a 16-bit encoding to an 8-bit
encoding is obviously truncation.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/rust-mozjs/208)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Nov 5, 2015

💔 Test failed - travis

@eefriedman
Copy link
Contributor Author

eefriedman commented Nov 5, 2015

The build timed out during the Spidermonkey build. The configuration probably needs to be changed somehow.

@jdm
Copy link
Member

jdm commented Nov 5, 2015

@bors-servo: retry

@bors-servo
Copy link
Contributor

bors-servo commented Nov 5, 2015

Testing commit 9c4a2ba with merge 0aae51a...

bors-servo added a commit that referenced this pull request Nov 5, 2015
Error reporting strings are Latin1, not UTF-8.

Because the best way to convert from a 16-bit encoding to an 8-bit
encoding is obviously truncation.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/rust-mozjs/208)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Nov 5, 2015

💔 Test failed - travis

@jdm
Copy link
Member

jdm commented Nov 5, 2015

Bah, we're being bitten by libc here. #211 needs to merge first.

@jdm
Copy link
Member

jdm commented Nov 5, 2015

@bors-servo: retry

bors-servo added a commit that referenced this pull request Nov 5, 2015
Error reporting strings are Latin1, not UTF-8.

Because the best way to convert from a 16-bit encoding to an 8-bit
encoding is obviously truncation.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/rust-mozjs/208)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Nov 5, 2015

Testing commit 9c4a2ba with merge e2458c4...

@bors-servo
Copy link
Contributor

bors-servo commented Nov 5, 2015

☀️ Test successful - travis

@bors-servo bors-servo merged commit 9c4a2ba into servo:master Nov 5, 2015
1 check passed
1 check passed
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.