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

Updated to branch 3071 revision 1640 #35

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Updated to branch 3071 revision 1640 #35

wants to merge 4 commits into from

Conversation

alkemir
Copy link

@alkemir alkemir commented Jun 19, 2017

Tested and working on Windows 32 and 64 bits.

@cztomczak
Copy link
Owner

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();
Copy link
Owner

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.

Copy link
Author

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
Copy link
Owner

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?

Copy link
Author

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.

Copy link
Owner

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
Copy link
Owner

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:

  1. Install mingw:
    a) If using 32-bit ...
    b) If using 64-bit ...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do

@cztomczak
Copy link
Owner

The review comments did not appear until now, had to click yet another button, sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants