letsfindaway / OpenBoard

I'm using this fork to contribute features and fixes to the upstream project. In order to create good pull requests, I'm rebasing my feature branches, squashing and reordering commits, etc. If you fork this repository be aware that my development branches may rewrite history without prior notice.
http://openboard.ch/
GNU General Public License v3.0
9 stars 0 forks source link

A `target="_blank"` link won't open in a widget #115

Closed kaamui closed 2 years ago

kaamui commented 2 years ago

Found by the test team. A target=_blank link won't open after a page has been embed in a widget.

I think it's potentially systematic, as long as a link is supposed to open in a new tab, but I wonder how we could intercept it to ignore blank attribute.

letsfindaway commented 2 years ago

Interception works probably by overwriting createWindow in the UBWebEngineView. In the Browser, this is already done in WebView. The widgets use a different view, and here this function is currently not reimplemented. However I'm not sure whether we could return this from here. A first test created a SIGSEGV when trying to do this.

According to https://doc.qt.io/qt-6/qwebenginepage.html#createWindow a newWindowRequested() signal is emitted when we don't reimplement this function. That would be an option, however only from Qt 6.2 upward.

I will try to find a solution!

letsfindaway commented 2 years ago

The following overload of UBWebEngineView::createWindow works for me:

QWebEngineView *UBWebEngineView::createWindow(QWebEnginePage::WebWindowType type)
{
    if (type == QWebEnginePage::WebBrowserTab) {
        // create a throwaway view to get the URL
        QWebEngineView* view = new QWebEngineView();

        connect(view, &QWebEngineView::urlChanged, this, [this,view](const QUrl& url){
            // load URL in current view and delete temporary view
            load(url);
            view->deleteLater();
        });

        return view;
    }

    return nullptr;
}
letsfindaway commented 2 years ago

closed as PR was merged