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

QWebEngine migration #1421

Open
pbek opened this issue Nov 15, 2019 · 15 comments
Open

QWebEngine migration #1421

pbek opened this issue Nov 15, 2019 · 15 comments

Comments

@pbek
Copy link
Owner

pbek commented Nov 15, 2019

@Waqar144, here we can talk about migration to QWebEngine (or adding it optionally).

My current issues with it are:
breaking compatibility with older Qt versions (and thus excluding quite some Linux distributions), not easy to integrate (I still failed to make it work with AppVeyor and TravisCI for macOS), limited or no printing and PDF generating support, much higher memory, cpu and diskspace consumption, ...

@Waqar144
Copy link
Contributor

Waqar144 commented Nov 15, 2019

@Waqar144, here we can talk about migration to QWebEngine (or adding it optionally).

My current issues with it are:
breaking compatibility with older Qt versions (and thus excluding quite some Linux distributions), not easy to integrate (I still failed to make it work with AppVeyor and TravisCI for macOS), limited or no printing and PDF generating support, much higher memory, cpu and diskspace consumption, ...

Memory and cpu will definitely take a hit.

Performance will still be good though, it's working well for me.

Printing might cause some big problems for a lot of distributions especially the ones < Qt 5.6

Edit: Printing support starts at Qt 5.7

@Waqar144
Copy link
Contributor

Waqar144 commented Nov 15, 2019

Printing is looking like this using the printToPdf() function from QWebEnginePage:

image

While the actual preview is like this:

image


For some reason, colors are being ignored however the rest of the CSS gets rendered correctly.

This function is only available for Qt 5.8 and above. There is another method, print(), however printed result looks like crap and the text is not selectable.

There is another method for versions lower than Qt 5.8: toHtml(), we can take that html and use already in place printing methods to print. Printed result is however plain html with no css rendering.

Qt 5.3 is a no go.

A solution that might work would be to use the old previewing system for debian 8 and the like and QWebEngineView for everything else.

Mac situation is still unclear.

@pbek
Copy link
Owner Author

pbek commented Nov 15, 2019

Almost nice :)

@Waqar144
Copy link
Contributor

Waqar144 commented Nov 16, 2019

Got the colors in print, Export to pdf working pretty good.

@pbek
Copy link
Owner Author

pbek commented Nov 16, 2019

Nice, on which Qt version?

@Waqar144
Copy link
Contributor

The latest version. This would work with everything above Qt 5.8

However there are some problems. It seems that in release version, Qresource doesn't load Javascript file. You have to manually add the Javascript using file:///
Without that, there's not much advantage in using it.

I have been considering all of this, whether all this effort is worth it or not?
We have good html rendering already. It is lightweight and is working pretty well on all kinds of systems. What are the advantages of using Qwebenginewidgets over Qtextbrowser?

  • ability to use Javascript to render code blocks
  • maybe a little better html rendering
  • full css support(we don't need very complex css support, since the generated html is very simple)

However the disadvantages are many:

  • more cpu/memory/disk space
  • having a full browser embedded in QON. (There wouldn't be much of a difference between us and electron then, would there?
  • No support for older Qt versions, meaning different code paths for different versions which would lead to increase in code and code complexity
  • Situation on macOS front is still unclear
  • complexity in running Javascript, we would have to use - -disable-web-security and --allow-local-file-access which may or may not bring security problems into the app. Not sure about this one.

It doesn't seem worth it.

For code highlighting I have another idea:

  • we do the code highlight in hoedown using span and keyword tables for different languages.
  • That should work and provide basic highlight support. This would also allow for custom styling using css.

@pbek
Copy link
Owner Author

pbek commented Nov 17, 2019

Thank you for your investigation!

Yes, I've a long list of feature requests and even some QTextBrowser rendering bugs that only could be solved with a "real" browser. But still there are so many disadvantages like older Qt versions and the resources needed for fully fledged browser... I'm really torn in that matter.

we do the code highlight in hoedown using span and keyword tables for different languages.

do you mean not using <code>?

@Waqar144
Copy link
Contributor

Waqar144 commented Nov 17, 2019

Thank you for your investigation!

Yes, I've a long list of feature requests and even some QTextBrowser rendering bugs that only could be solved with a "real" browser.

May I know some of these bugs? Rendering for me has been working well though I admit my notes are usually very simple.

But still there are so many disadvantages like older Qt versions and the resources needed for fully fledged browser... I'm really torn in that matter.

The resources are not that much of an issue. Even with QWebengineView, the resource consumption is much less than an electron based app or a full browser.

There's still the older QWebengine, that'd work well but it has been deprecated although it still works and is present in many Qt distributions.

And there is one more solution, the hardest one I guess. We render the html ourselves, using our own widget and doing all the painting ourselves. But even as I write this, I can imagine the number of regressions happening and edge cases that'd need to be handled.

do you mean not using <code>?

I mean using <span> inside <code> blocks, to give different colors and font weight to words.

@r00tr4v3n
Copy link
Contributor

r00tr4v3n commented May 25, 2020

Interesting topic. @Waqar144 already mentioned many of my wishes and concerns.
I'm not yet clear on what the final goal is though:

  • electron differences -> QON would still have a Qt backend
  • Why JavaScript? Just to render codeblocks/mermaid etc? -> This could be accomplished differently too (e. g. PlantUML/QML). A simple browser base for an extended html/css support might suffice to reduce small render bugs and improve the display quality/resolution (Preview (image) resolution and SVG support #1772). This would also reduce security concerns.
  • older systems -> As a Qt novice, I have to ask: Would it be possible to use the old Webkit as standalone embedded browser or is it deprecated and unusable on new systems? The results seem pretty compact (qtweb).

@Waqar144
Copy link
Contributor

Waqar144 commented Jun 5, 2020

Interesting topic. @Waqar144 already mentioned many of my wishes and concerns.
I'm not yet clear on what the final goal is though:

Final goal is quite simple, to have a better preview to fix issues with checkboxes, lists and there are other issues for e.g., rendering issues with RTL text.

  • Why JavaScript? Just to render codeblocks/mermaid etc? -> This could be accomplished differently too (e. g. PlantUML/QML). A simple browser base for an extended html/css support might suffice to reduce small render bugs and improve the display quality/resolution (Preview (image) resolution and SVG support #1772). This would also reduce security concerns.

Yes, QML can handle somethings but the vast majority of things would probably run better in a FULL browser think.

  • older systems -> As a Qt novice, I have to ask: Would it be possible to use the old Webkit as standalone embedded browser or is it deprecated and unusable on new systems? The results seem pretty compact (qtweb).

QWebEngine was based on webkit I think, but they deprecated that for Chromium. It is being maintained here currently, that's the lightest option we have right now. Maybe there are others but I don't know about them.

@Beurt
Copy link

Beurt commented Dec 8, 2022

Hi,

Are you still considering the QWebEngine migration?

Since a few months I am using https://github.com/marp-team/marp for my presentations and I write my Markdown in Qownnotes because (1) I'm used to it; (2) It is my main tool to write anything; (3) It is (one of ?) the best Markdown editor I ever tested (and I tested A LOT).

As I wanted to have a preview of the slides in the QownNotes preview (like the Marp VS Code plugin), I wrote a script (code below) to make https://github.com/marp-team/marp-cli render the Markdown. Unfortunately the generated HTML preview is pretty ugly (QTextBrowser ?). Apparently a lot of the CSS are not used.

With the actual preview, it may be impossible to implement such script.


The code of the script is inspired from https://github.com/qownnotes/scripts/tree/master/render-plantuml

It uses startDetachedProcess to render the markdown with marp-cli, then push the HTML result to the note preview HTML.

import QtQml 2.0

/**
 * This script renders slide from markdown with Marp-cli
 *
    * Dependency: https://github.com/marp-team/marp-cli
    *
 */
QtObject {
    property string marpPath;
    property string workDir;
    property string additionalParams;

    // register your settings variables so the user can set them in the script settings
    property variant settingsVariables: [
        {
            "identifier": "marpPath",
            "name": "Path to marp-cli binary file",
            "description": "Please enter absolute path to marp-cli file:",
            "type": "file",
            "default": "/usr/local/bin/marp"
        },
        {
            "identifier": "workDir",
            "name": "Working directory",
            "description": "Please enter a path to be used as working directory i.e. temporary directory for file creation:",
            "type": "file",
            "default": ""
        },
        {
            "identifier": "additionalParams",
            "name": "Additional Params (Advanced)",
            "description": "Enter any additional parameters you wish to pass to marp. This can potentially cause unexpected behaviour:",
            "type": "string",
            "default": ""
        }
    ];
    function marp(html,note) {
        var filePath = script.toNativeDirSeparators(script.getPersistentVariable("renderMarp/workDir") + "/" + note.id);

        // Check for cached identical note
        var cached = "notCached";
        var oldContent = "";
        var mdExists = script.fileExists(filePath);
        var htmlExists = script.fileExists(filePath + ".html");
        if(mdExists && htmlExists){
            oldContent = script.readFromFile(filePath);
            if (Qt.md5(oldContent) == Qt.md5(note.noteText))
                cached = "cached";
        }
        script.log(cached);
        // if not cached
        if (cached == "notCached") {
            script.writeToFile(filePath, note.noteText);
            // Launch Marp-cli
            const d = new Date();
            var noteId = script.getPersistentVariable("renderMarp/noteId");
            script.setPersistentVariable("renderMarp/currentTimeStamp", d.getTime());
            var timeStamp = script.getPersistentVariable("renderMarp/currentTimeStamp");
            var params = [
                    "--template", "bare",
                    filePath,
                    "-o", filePath+".html"
                    ];
                    
            // Launch html renderding with marp-cli
            var result = script.startDetachedProcess(
                    marpPath,
                    params,
                    "marp-callback-" + noteId + "-" + timeStamp,
                    0,
                    null,
                    script.toNativeDirSeparators(script.getPersistentVariable("renderMarp/workDir")));
            script.setPersistentVariable("renderMarp/marpRunning/" + noteId, "running");
            if (!result) {
                var error = script.getLastError();
                script.log("Error: Failed to launch marp process: " + error);
            }
            script.log("launching Marp: " + noteId + "-" + timeStamp);
            
            // Here, we don't have teh rendered html yet
            // We dont have a valid cache
            // but if there is an old one we return it as preview
            // waiting for the renderding to finsh
            if (htmlExists) {
                html = script.readFromFile(filePath + ".html");
            }
        } else {
            // If there is a cache, we read it.
            html = script.readFromFile(filePath + ".html");
        }
        return html;
    }

    function onDetachedProcessCallback(callbackIdentifier, resultSet, cmd, thread) {
        script.log(cmd);
        script.log("resultSet: " + resultSet);
        script.log("thread: " + thread);
        var noteId = script.getPersistentVariable("renderMarp/noteId");
        if (callbackIdentifier
                ==
                "marp-callback-" + noteId + "-" + script.getPersistentVariable("renderMarp/currentTimeStamp")
            ) {
            script.log("entering callback: " + noteId + "-" + script.getPersistentVariable("renderMarp/currentTimeStamp"));
            // If the flag is not set to done, then refresh
            if (script.getPersistentVariable("renderMarp/marpRunning/" + noteId) != "done") {
                script.setPersistentVariable("renderMarp/marpRunning/" + noteId, "done");
                script.regenerateNotePreview();
                script.log(`refresh`);
            } else {
                // else, reset the flag for the next modification
                script.setPersistentVariable("renderMarp/marpRunning/" + noteId, "");
            }
        }
    }
    /**
     * This function is called when the markdown html of a note is generated
     *
     * It allows you to modify this html
     * This is for example called before by the note preview
     *
     * The method can be used in multiple scripts to modify the html of the preview
     *
     * @param {NoteApi} note - the note object
     * @param {string} html - the html that is about to being rendered
     * @param {string} forExport - the html is used for an export, false for the preview
     * @return {string} the modified html or an empty string if nothing should be modified
     */
     function noteToMarkdownHtmlHook(note, html, forExport) {
        script.log("launch");
        script.log("flag is: " + script.getPersistentVariable("renderMarp/marpRunning/" + note.id));
        script.setPersistentVariable("renderMarp/workDir", workDir ? workDir: script.cacheDir("render-marp"));
        script.setPersistentVariable("renderMarp/noteId", note.id);
        var frontmatter = note.noteText.split("---")[1];
        if (frontmatter && frontmatter.indexOf("marp: true") >= 0) {
            // render the markdown as slides using Marp
            html = marp(html,note);
        }
        return html;
    }
}

@Beurt
Copy link

Beurt commented Dec 8, 2022

Here is a screenshot of the rendering of the maprit presentation example: https://raw.githubusercontent.com/yhatt/marp-cli-example/master/PITCHME.md (it is supposed to look like this: https://yhatt.github.io/marp-cli-example/) in QownNotes (with the previous script):

Screenshot_20221208_104908

@Waqar144
Copy link
Contributor

Waqar144 commented Dec 8, 2022

I would say no, QWebEngineView is not really an option as it is hard to integerate it and can sometimes cause weird issues. It also has a massive size overhead.

@pbek
Copy link
Owner Author

pbek commented Dec 8, 2022

I fully concur with @Waqar144 statement.

@Beurt
Copy link

Beurt commented Dec 8, 2022

OK. Thank you for your answers. I will try a different approach for the script (maybe using the -p or -s options of marp-cli).

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

No branches or pull requests

4 participants