-
Notifications
You must be signed in to change notification settings - Fork 58
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
Update WebKitGTK to 2.20.5 #179
base: develop
Are you sure you want to change the base?
Conversation
Fantastic - thanks very much. @Stivius I've requested a review from you please, when you have time. |
@bauerj thanks for this PR! What we can do is ask them to implement similar functionality in the upcoming release. |
@Stivius - you're right of course, sorry I forgot about this point! Did we raise an issue in the WebKitGTK project for this? I will forget again unless we have it written down, so I have opened an issue for it and linked to this PR. |
Indeed, this seems to be a regression in WebKitGTK. From reading the docs I assume this use-case should be possible but testing confirmed that it doesn't work. Did you test this with a different version of WebKitGTK? Maybe the bug was already solved. |
@bauerj I've tested with 2.28 which seems to be the latest release. So I think we need to create an issue on their tracker. |
Yes, please do. I'm curious if you were able to build the webkitgtk "minibrowser" executable. It seems to have a command line option for the background color, so if it shows the same behaviour it would probably be a lot easier for them to reproduce this issue. |
@Stivius any updates? Did you create an issue on their bug tracker? If so, can you link it here? |
@Stivius - can we confirm whether or not this happened?
Thanks! |
For what it's worth, I have successfully built R5 with Webkit2Gtk upgraded to version 2.26.4. Although the docker build does run into linking errors (undefined reference) related to Webkit2Gtk unless one specifies the webkit2gtk dependency in the player/control/media/CMakeLists.txt instead of the player root folder's CMakeLists.txt. I spent several days figuring this out as I'm unfamiliar with CMake. |
Does successfully mean you didn't run into the transparency bug? |
I haven't encountered the bug so far as described by Stivius - Ill try if I can reproduce it next week with a test layout. BTW: there is this bit in the documentation regarding _set_background_color, not sure if its needed for transparency: "Note that the parent window must have a RGBA visual and “app-paintable” property set to TRUE for backgrounds colors to work." ( Source: https://webkitgtk.org/reference/webkit2gtk/stable/WebKitWebView.html#webkit-web-view-set-background-color ) |
Thank you for both your comments and continued interest in this PR. If you want to test the transparency issue, all you need to do is put some web content on top of an image or video. You will see the web content blocks the content underneath. If this has been fixed between 2.20.5 and 2.26.4 it would be a fantastic thing and we'd happily accept the contribution! |
I'll be sure to post my changes if it's indeed working. Didn't have time today at work :) |
Yes, it appears indeed that using WebKit2GTK 2.26.4 at least fixes this. Here's a short demonstration of a PNG file with fully transparent background overlaid over a playing video (https://drive.google.com/file/d/1_Khx6pPqOWCK41FO0FdK1OT0CFWJ1Ykn/view?usp=sharing 30 MiB .mp4) So I suggest the original author to include that version instead of the older one. |
Sure, I will update this PR to 2.28.4 later. Thanks for testing! |
@bauerj where do you use the webview in your example? You said you put PNG image over video but we need to test some web content as well. |
What example do you mean? |
Thanks for the updated PR, excited to try this. Unfortunately this doesn't build with the change to core20 (gnome-3-28 is not supported by core20), and doesn't build with Below is the build error with
Below is the build error with
The PNG must be in a browser? |
The example provided by @jthartika shows a PNG image over the top of a Video as a demonstration of the transparency working... however Xibo doesn't show images in a web browser (unless they are embedded with an Embedded/Web Page widget). I'm very happy to test this if I can get it built - I can then run a variety of different transparency tests to confirm. |
Apologies @dasgarner, this isn't quite ready yet. I don't have a snapcraft environment to test here. Once it builds, I will let you know that it can be tested. |
Great, thank you very much! |
Let's make sure we are testing the same thing. I've prepared a small example which works fine with 2.4.10 and doesn't work with higher versions. The example consists of an image and webview on top of it. You need to build using CMake and adjust paths for image and |
I've created the bug report so you can track the status of this issue |
This updates the included WebKitGTK version from 2.4.10 (released 2016-03-14) to 2.20.5 (released 2018-08-13).
Newer is not always better but a lot has changed in the web in the last 4 years. For example, xibo-linux now supports flexboxes in HTML layouts which previously didn't work properly. Arguably the new version is not exactly "state of the art" either. When Ubuntu 20.04 is released, the snap base should be changed accordingly to pull in an even newer version.
The API has changed a bit but luckily the changes in this project are marginal.