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

Enable evaluating javascript in the firefox devtools console #23296

Merged
merged 1 commit into from May 9, 2019

Conversation

@codehag
Copy link
Contributor

@codehag codehag commented May 1, 2019

This enables one to use the Firefox Devtools console to evaluate things in servo.

Steps:
start servo: ./mach run tests/html/about-mozilla.html -d --devtools 6000
start firefox and go to Tools -> Web Developer -> Connect...
Set the port to connect to as 6000
Try evaluating things in the console.

Screenshot 2019-05-01 at 13 02 37


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #___ (GitHub issue number if applicable)
  • There are tests for these changes OR
  • These changes do not require tests because ___

This change is Reviewable

@codehag codehag force-pushed the evaluate-js-fix branch from 3170f53 to cba98c6 May 1, 2019
@codehag codehag changed the title Introduce EvaluateJSAsync method and evaluationResponse event Enable evaluating javascript in the firefox devtools console May 1, 2019
@@ -89,6 +108,86 @@ pub struct ConsoleActor {
pub streams: RefCell<Vec<TcpStream>>,
}

impl ConsoleActor {
fn evaluateJS(
Copy link
Contributor Author

@codehag codehag May 1, 2019

This function is moved as-is from the "evaluateJS" handler so that "evaluateJS" and "evaluateJSAsync" can share it

input: input,
result: result,
timestamp: 0,
exception: Value::Null,
Copy link
Contributor Author

@codehag codehag May 1, 2019

Exceptions must be null if they are not present. We were sending an empty object over the wire, and devtools was interpreting it as an error.

// for an async event with the resultID
stream.write_json_packet(&early_reply);
let reply = self.evaluateJS(&registry, &msg).unwrap();
let msg = EvaluateJSEvent {
Copy link
Contributor Author

@codehag codehag May 1, 2019

I hope there is a better way to do this..

Copy link
Member

@Manishearth Manishearth May 1, 2019

You could have an intermediate type with From impls but I don't think it's a big deal.

Value::Object(m)
},
"evaluateJSAsync" => {
//TODO: use a timestamp for resultID
Copy link
Member

@Manishearth Manishearth May 1, 2019

Maybe just use a uuid? A monotonic id works too.

Copy link
Member

@Manishearth Manishearth left a comment

looks pretty good! If you can initialize the result id with a monotonic counter or uuid we can land this!

@codehag
Copy link
Contributor Author

@codehag codehag commented May 2, 2019

looks pretty good! If you can initialize the result id with a monotonic counter or uuid we can land this!

I tried this with a UUID, but I ran into an issue: the compiler complains and says that it cannot resolve module or type "uuid". I took a look around the codebase how this is imported elsewhere and I am not sure what I am doing wrong. @Manishearth Do you have any suggestions?

@Manishearth
Copy link
Member

@Manishearth Manishearth commented May 7, 2019

You'll need to add the uuid crate to devtools/Cargo.toml, and extern crate uuid in devtools/lib.rs

@codehag
Copy link
Contributor Author

@codehag codehag commented May 7, 2019

great, all set!

@Manishearth
Copy link
Member

@Manishearth Manishearth commented May 7, 2019

Mind squashing your commits? r=me

@codehag codehag force-pushed the evaluate-js-fix branch from 0ff2dd8 to 8d8f482 May 7, 2019
@KiChjang
Copy link
Member

@KiChjang KiChjang commented May 9, 2019

@bors-servo r=Manishearth

@bors-servo
Copy link
Contributor

@bors-servo bors-servo commented May 9, 2019

📌 Commit 8d8f482 has been approved by Manishearth

@bors-servo
Copy link
Contributor

@bors-servo bors-servo commented May 9, 2019

Testing commit 8d8f482 with merge e39b2b9...

bors-servo added a commit that referenced this issue May 9, 2019
Enable evaluating javascript in the firefox devtools console

This enables one to use the Firefox Devtools console to evaluate things in servo.

Steps:
start servo: ./mach run tests/html/about-mozilla.html -d --devtools 6000
start firefox and go to Tools -> Web Developer -> Connect...
Set the port to connect to as 6000
Try evaluating things in the console.

<img width="901" alt="Screenshot 2019-05-01 at 13 02 37" src="https://user-images.githubusercontent.com/26968615/57015054-0aa0ce80-6c13-11e9-8031-9299f2da5dba.png">

---
<!-- 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
- [ ] These changes fix #___ (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- 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/23296)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

@bors-servo bors-servo commented May 9, 2019

@bors-servo bors-servo merged commit 8d8f482 into servo:master May 9, 2019
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants