Creating a mixin class to support both WebKit and WebEngine

[jonoomph]

@ferdnyc What do you think as a short term solution to support both Webkit and Webengine? I just threw this together quickly as a demonstration of what might work.

[jonoomph]

Turns out we'll also need to make some JS changes to support both webkit and webengine, which you might have already been looking into. Just sharing for my own amusement. =)

[ferdnyc]

@jonoomph

Hmm. I have to wrap my head around this. I don't have a lot of familiarity with mixins (which are sort of the exact opposite of abstraction, really) — I've never had much call to use them.

And I don't think it's necessary here, either. If you just want to do fallback replacement of the base class, Python provides that in a much easier form with module aliasing:

try:
    from PyQt5.QtWebEngineWidgets import QWebEngineView as WebViewParent
    web_view_implementation = "WebEngine"
except ImportError:
    try:
        from PyQt5.QtWebKitWidgets import QWebView as WebViewParent
        web_view_implementation = "WebKit"
     except ImportError:
         raise Exception("Could not find supported web backend!", exc_info=1)

class TimelineWebView(WebViewParent, updates.UpdateInterface):
    ...

In terms of the overridable QtWidgets APIs that we subclass the views to reimplement, the two WebFooView classes should be totally interchangeable.

Of course, that becomes even easier if the backend communication isn't part of the view (as it shouldn't be, anyway), which is where a timeline model class still comes in handy.

But that's if you just want the code to pick up whatever web implementation it can find, to use as the base class. I was thinking of an abstract implementation where the backend could even be user-selectable (at least in theory), on systems where both backends are available — like any recent Linux distro, for example.

That's where a PIMPL-style implementation comes into play, since our timeline class would gain a self.view property holding a reference to whatever view object is in use. (Much like the self.model reference in the view classes abstracts away the details of their model implementation, and makes it so easy to customize the data presented to the view by interposing proxy model instances between the view and the base model.)

However, if I'm understanding them correctly, mixins might actually be useful in that scenario, to solve the problem of needing to override all the same methods in whichever view gets chosen. The API customizations we normally perform by subclassing could theoretically become a mixin (or mixins) applied to the Qt-standard view classes (rather than custom subclasses) in a write-once-sprinkle-liberally sort of way, instead of all the rote (and repetitive) method-overriding — which of course is Qt's norm, since C++ is pretty much incompatible with the mixin pattern.

In fact, if we were to build (say) a DragHandler mixin that encapsulates a slightly-more-generalized form of our normal set of dragEnterEvent(), dropEvent(), dragMoveEvent(), dragLeaveEvent() overrides, we could apply it to every class where we currently end up cut-and-pasting nearly identical code — which is at least the Files view (both of them), the Timeline (now also both of them), and possibly some others (Properties TableView?) as well. That could potentially be quite cool.

Turns out we'll also need to make some JS changes to support both webkit and webengine, which you might have already been looking into. Just sharing for my own amusement. =)

Hm, bummer. ...Is the WebEngine-style JS definitely incompatible with WebKit? I was hoping they'd play nice.

It's worth noting that QtWebChannel is present in all recent releases/builds of Qt, and is not in any way dependent on QtWebEngine — so hopefully we can leave the new WebChannel communication plumbing in place, even when legacy WebKit is the backend.

If we're using MSYS2 PyQt5 packages, we don't have to settle for legacy Qt 5.5 or whatever — their current (5.15.0) Qt and PyQt5 packages include QtWebKit still, and definitely include QtWebChannel as well. Only QtWebEngine is missing.

I was hoping to approach it from the assumption that we can let the QtWebChannel stuff ride regardless which backend is loaded, and see how that worked out. (Maybe badly, though?)

[jonoomph]

I don't see anyway to call self.page().setWebChannel(self.webchannel) with QWebView, and without it, the JS seems to break. I'll keep looking though.

[jonoomph]

I am experimenting with calling setHtml (instead of setUrl), so we can make some substitutions, such as including a different mixin-BACKEND.js in the index.html. That would allow us to initialize things correctly for each backend... in theory.

[jonoomph]

@ferdnyc Regarding aliasing, we are essentially doing that here (at the end of the timeline_mixins.py), where we return 1 class or the other. However, either way, we need to have 2 classes with some slight differences between them. And yes, mixin classes could really help us remove lots of duplicate code, and allow us to more easily style widgets, etc...

Take a look at the most recent commit. I've inserted in a JS file, based on the backend (WebEngine or WebKit). Each one does something slightly different to initialize things. The WebEngine version must define a global timeline variable, while the WebKit version must not have a global timeline var, or it overrides the way we are attaching Qt. This took some trial and error and patience to work through.

I was also able to add custom JS logging for the WebKit version, but that same approach failed with WebEngine (it always turned the page white and blank with no logging at all). So, I left the WebEngine version without custom logging.

As far as I can tell, this PR successfully supports both WebEngine and WebKit fully.

[jonoomph]

@ferdnyc Here is my plan. I'm going to merge this into the new-webengine-support branch, and then merge all the new-webengine-support PRs (libopenshot-audio, libopenshot, and openshot-qt). This will do a few great things:

1. Get our Mac daily builds working again (finally)
2. Move to our new AppImage builds on Ubuntu 18.04 Bionic (newer Qt, Web Engine, perhaps better hardware support)
3. Continue to work on Windows (due to these WebKit mixins)

Then, in the future, when one of us is so inclined... we can do a more extensive overhaul of the timeline, separating model and view, etc... That seems like a great plan to me!

[ferdnyc]

@jonoomph

Sorry, must've missed or spaced on these.

I don't see anyway to call self.page().setWebChannel(self.webchannel) with QWebView, and without it, the JS seems to break. I'll keep looking though.

Yeah, you can't — QWebChannel is present, but the built-in plumbing to wire it into the web view isn't there with QtWebKit like it is with QtWebEngine.

We'd have to do the work ourselves of connecting up the QWebChannel over a QAbstractDataTransport of some sort (QWebSockets?) — basically, all of the things that QWebEngineView does automatically behind the scenes. TBH I have no idea how much effort that would involve, but at least we've got Qt's own QtWebEngine code to provide an example of how it works.

I was also able to add custom JS logging for the WebKit version, but that same approach failed with WebEngine (it always turned the page white and blank with no logging at all). So, I left the WebEngine version without custom logging.

Looks like it broke in the QtWebEngine version because the page() object there is a QWebEnginePage, so you have to inherit from that class if you want to use a subclass in its place. These admittedly-hackish changes got it working for me with both backends interchangeably (though among other issues, they'd break if run in a "no backends available" scenario):

diff --git a/src/windows/views/timeline_mixins.py b/src/windows/views/timeline_mixins.py
index 2a553c40..92e05ef4 100644
--- a/src/windows/views/timeline_mixins.py
+++ b/src/windows/views/timeline_mixins.py
@@ -37,6 +37,7 @@ from functools import partial
 try:
     # Attempt to import QtWebEngine
     from PyQt5.QtWebChannel import QWebChannel
+    from PyQt5.QtWebEngineWidgets import QWebEnginePage as WebPageClass
     from PyQt5.QtWebEngineWidgets import QWebEngineView
     IS_WEBENGINE_VALID = True
 except ImportError:
@@ -45,11 +46,12 @@ except ImportError:
 
 try:
     # Attempt to import QtWebKit
-    from PyQt5.QtWebKitWidgets import QWebView, QWebPage
+    from PyQt5.QtWebKitWidgets import QWebView
+    if not IS_WEBENGINE_VALID:
+        from PyQt5.QtWebKitWidgets import QWebPage as WebPageClass
     IS_WEBKIT_VALID = True
 except ImportError:
     QWebView = object # Prevent inheritance errors
-    QWebPage = object
     IS_WEBKIT_VALID = False
 
 
@@ -143,11 +145,14 @@ class TimelineQtWebEngineMixin(TimelineBaseMixin, QWebEngineView):
             event.ignore()
 
 
-class LoggingWebPage(QWebPage):
+class LoggingWebPage(WebPageClass):
     """Override console.log message to display messages"""
     def javaScriptConsoleMessage(self, msg, line, source):
         log.warning('JS: %s line %d: %s' % (source, line, msg))
 
+    def __init__(self):
+        super().__init__()
+
 class TimelineQtWebKitMixin(TimelineBaseMixin, QWebView):
     """QtWebKit Timeline Widget"""
 

[ferdnyc]

These admittedly-hackish changes got it working for me with both backends interchangeably

Well, I take that back... I accidentally left out of my diff the part where I do:

self.setPage(LoggingWebPage())

In the QWebEngineView __init__(), but even though it is there, it doesn't appear that it's actually having any effect. GammaRay says the object is still an instance of a stock QWebEnginePage. Hmmm.

[ferdnyc]

Ah, here we go. I needed to make the LoggingWebPage accept a parent argument, and use that to parent it to the QWebEngineView when creating it. Now that I've done that, it's properly parented and I get this message on the console — "progress?"

  exceptions:ERROR Unhandled Exception
TypeError: javaScriptConsoleMessage() takes 4 positional arguments but 5 were given

[ferdnyc]

Ohhh-kay. Bit of a delay there, I went on a detour exploring a logging idea I had. I wanted to see if I could get the logs coming from JS to display the JavaScript filename in place of the Python module name, when the messages are logged.

The answer, should it ever come up, is: No. Not without it requiring way more effort than would be worth it, anyway.

So, I gave up on that, but my PR #3681 contains changes (significantly less hackish than the above) to set up logging in either web backend. The QWebEnginePage logging did indeed add another parameter to the calls, a log level — they now have three of them. So, those will get mapped to the appropriate log.INFO, log.WARNING, and log.ERROR Python logging level.