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

Decode UTF-8 with from_utf8_lossy in DedicatedWorkerGlobalScope #13306

Merged
merged 1 commit into from Sep 19, 2016

Conversation

@Jenselme
Copy link
Contributor

Jenselme commented Sep 18, 2016


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #13247
  • There are tests for these changes: ./mach test-wpt /workers/semantics/encodings/004.worker passes

This change is Reviewable

@highfive
Copy link

highfive commented Sep 18, 2016

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @larsbergstrom (or someone else) soon.

@highfive
Copy link

highfive commented Sep 18, 2016

Heads up! This PR modifies the following files:

  • @KiChjang: components/script/dom/dedicatedworkerglobalscope.rs
Copy link
Contributor

Ms2ger left a comment

Thanks! Just two small comments.

@@ -173,7 +173,7 @@ impl DedicatedWorkerGlobalScope {

let roots = RootCollection::new();
let _stack_roots_tls = StackRootTLS::new(&roots);
let (url, source) = match load_whole_resource(LoadContext::Script,
let (metadata, bytes) = match load_whole_resource(LoadContext::Script,

This comment has been minimized.

Copy link
@Ms2ger

Ms2ger Sep 19, 2016

Contributor

Nit: realign the other arguments to load_whole_resource so they line up again.

@@ -1,4 +1,4 @@
[004.worker]
type: testharness
bug: https://github.com/servo/servo/issues/13247
expected: CRASH
expected: PASS

This comment has been minimized.

Copy link
@Ms2ger

Ms2ger Sep 19, 2016

Contributor

This file can be removed entirely now.

@Jenselme
Copy link
Contributor Author

Jenselme commented Sep 19, 2016

@Ms2ger Just updated the PR to fix you comments.

@Ms2ger
Copy link
Contributor

Ms2ger commented Sep 19, 2016

Thanks so much!

@bors-servo r+

@bors-servo
Copy link
Contributor

bors-servo commented Sep 19, 2016

📌 Commit 9adda6e has been approved by Ms2ger

@bors-servo
Copy link
Contributor

bors-servo commented Sep 19, 2016

Testing commit 9adda6e with merge fe426f6...

bors-servo added a commit that referenced this pull request Sep 19, 2016
…Ms2ger

Decode UTF-8 with from_utf8_lossy in DedicatedWorkerGlobalScope

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #13247

<!-- Either: -->
- [X] There are tests for these changes: `./mach test-wpt /workers/semantics/encodings/004.worker` passes

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13306)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Sep 19, 2016

@bors-servo bors-servo merged commit 9adda6e into servo:master Sep 19, 2016
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
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.

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