Migrate to KCEF #25
Migrate to KCEF #25
Conversation
|
Great job! KCEF successfully addressed previously unsolvable issues with JCEF. I greatly appreciate your hard work! However, I only have one concern, which is the compatibility and stability of KCEF. Although it is built on JCEF, we encountered compatibility issues during our initial testing. While it currently runs without problems on our machines, it may not be compatible with all platforms. Is it too early to directly integrate it into this project as a replacement for JCEF without sufficient developer usage? What if we create a separate artifact for it and label it as experimental? This way, developers can decide for themselves if they want to try the new feature. By doing this, we can test its compatibility and gather suggestions from developers. Do you have any other suggestions? Thanks for your contribution again! |
|
It should be fully stable as the compatibility issue was my fault because I deleted code that was not only deprecated but could not be compiled on non macOS platforms as well. But we could go for your experimental suggestion of course. |
|
@DatL4g Apologies for the delay in responding. I am currently working on other issues, such as local HTML file loading and removing unused logs. Once these issues are resolved, I will conduct more comprehensive testing of KCEF. Although it may take some time, I believe it can be completed by the end of this week. Thank you for your understanding. Additionally, I noticed that the basic HTML code loading is not functioning on your main branch currently. I suspect this may be due to your recent commit regarding HTML loading. Could you please have a look at this issue? |
fixed error that content may not load BREAKING CHANGE: this requires the user to listen for the init progress, however the previous implementation was not really different since it just returned null for KCEFClient and didn't wait for initialization anyway
|
@KevinnZou fixed content loading problem Adds a (somewhat) breaking change, view commit message. |
refactored previous merge conflict resolving commit
|
@DatL4g I apologize for the delayed response. I have successfully resolved the issue with loading local HTML files. It is now functioning properly on both Android and iOS devices. As for the desktop side, I think we can focus on the switch to KCEF first before implementing support for it. I will begin testing the pull request tomorrow and merge it promptly if there are no issues. Thank you for your patience! |
|
@DatL4g I have tested the main functionalities and everything is working well on my end. Good job! As for loading local HTML files on the desktop side, I have implemented a simple solution based on your suggestion in this comment. However, it is not merged into this branch yet and does not support external links like Android and iOS do. I am unsure if KCEF supports this feature. If not, I think we can just proceed with the previous solution for now and explore ways to support it later. Can you please take a look and provide your suggestion? |
|
Well technically CEF supports this, but it probably needs a custom resource handler. |
Sure, I have implemented the |
|
Not for now, it's ready to merge. As mentioned earlier we should link the KCEF documentation then (I will add a compose section, so people know the jvmArgs) and we have to state that the previous jcef-bundle folder has to be deleted, easiest way is to set the installDir to kcef-bundle |
Great! Could you please update this information in the README.desktop.md file before I merge this pull request? |
|
@KevinnZou quickly added documentation some days ago. |
I apologize for just now discovering that my review is still pending. I thought I had submitted it and waiting for your reply. |
The KCEF module is visible to the user.
Everything required to initialize is documented on my side so we just need to link it.
Delete previous bundle
When we create a new release we need to state that the previous jcef-bundle needs to be deleted, since it required the new JetBrains/jcef binaries