-
-
Notifications
You must be signed in to change notification settings - Fork 127
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
Updated to branch 3071 revision 1640 #35
base: master
Are you sure you want to change the base?
Conversation
The old PR for reference: #34 Added a few review comments. Otherwise looks good, thanks! |
// loop only when last browser is closed. Otherwise | ||
// closing a popup window will exit app while main | ||
// window shouldn't be closed. | ||
cef_quit_message_loop(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This call should be commented out, otherwise popups will not work. In cef2go examples there are already explicit calls to cef.QuitMessageLoop()
. The cefcapi project use case is very minimal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will do
-------------- | ||
1. In cef_string.h: | ||
this => typedef cef_string_utf16_t cef_string_t; | ||
to => #define cef_string_t cef_string_utf16_t |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this no more required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No modification to the headers was required for compilation,
although I am not sure why it was required. My testing was
very shallow in the sense that I worried about being able to
start the browser and use it for my ends, I did not experiment with the API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this change in headers was required for Mac or Linux.
4. Run build.bat (or "build.bat noconsole" to get rid of the console | ||
window when running the final executable) | ||
|
||
5. You might need to help your linker find libcef.dll |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make only one "Getting started on Windows" section and just add a)
and b)
sublists for 32-bit and 64-bit. Something like:
- Install mingw:
a) If using 32-bit ...
b) If using 64-bit ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will do
The review comments did not appear until now, had to click yet another button, sorry. |
Tested and working on Windows 32 and 64 bits.