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

Browser::new() changes CEF follow up #17273

Closed
paulrouget opened this issue Jun 12, 2017 · 4 comments
Closed

Browser::new() changes CEF follow up #17273

paulrouget opened this issue Jun 12, 2017 · 4 comments
Labels
A-embedding C-assigned

Comments

@paulrouget
Copy link
Contributor

@paulrouget paulrouget commented Jun 12, 2017

Since #16477, we need to make some changes in CEF:

  • No need to use the shell.homepage pref
  • don't call frame.set_url in ServoCefBrowser::new

/cc @sadmansk
@paulrouget paulrouget mentioned this issue Jun 12, 2017
Merged
3 of 4 tasks complete
@sadmansk
Copy link
Contributor

@sadmansk sadmansk commented Jun 12, 2017 •
edited

@paulrouget I'm on it 👍
@sadmansk
Copy link
Contributor

@sadmansk sadmansk commented Jun 17, 2017

@paulrouget Hey, just had a quick inquiry. CefWrap::to_rust(*const cef_string_t) returns an &[u16] value, I have been searching around but I couldn't find a way to convert that to &str, what I found was how to convert from utf16 to std::String. Since ServoUrl::parse takes in a &str, that conversion is necessary. Any pointers on how to get from the original const cef_string_t to an &str that can be passed to ServoUrl::parse?
@jdm
Copy link
Member

@jdm jdm commented Jun 17, 2017

CEF strings are UTF-16, while Rust strings are UTF-8. You will need to convert it into a String then obtain the &str from that.
@sadmansk
Copy link
Contributor

@sadmansk sadmansk commented Jun 18, 2017

@jdm That's what I was going for. Thanks for the confirmation!
@sadmansk sadmansk mentioned this issue Jun 28, 2017
Merged
3 of 5 tasks complete
bors-servo added a commit that referenced this issue Jul 4, 2017
@bors-servo
Auto merge of #17548 - sadmansk:cef-update, r=paulrouget
Loading status checks…
b55b9dc 
Fix cef to follow new Browser::new() interface

Changes the call to `Browser::new()` in cef to contain the target_url.

cc @paulrouget
<!-- 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 #17273.

<!-- 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/17548)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-embedding C-assigned
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

Fix cef to follow new Browser::new() interface
3 participants
@jdm @paulrouget @sadmansk
You can’t perform that action at this time.