[webkit-qt] qweb{page, view} and qgraphicswebview createWindow method.

tonikitoo (Antonio Gomes) tonikitoo at gmail.com
Mon Oct 26 22:11:13 PDT 2009


Hi,

While working on bug 30771 ([Qt] Make qwebpage's createWindow not
qwebview dependent) and discussing a first attempt to improve the
current implementation w/ Kenneth today, it turns out that the current
QWebPage::createWindow implementation/documentation ( is bogus and )
have to be improved . See quote below:

QWebPage *QWebPage::createWindow(WebWindowType type)
{
  QWebView *webView = qobject_cast<QWebView *>(d->view);
  if (webView) {
    QWebView *newView = webView->createWindow(type);
    if (newView)
        return newView->page();
  }
  return 0;
}

* Implementation hardcodes a widget (QWebView for instance) dependency
here, which makes it poorly extensible to QGraphicsWebView.
* As it is protected, people usually reimplement it for its
application own needs, and it is ok. *However* QWebView also has a
protected ::createWindow method to be reimplemented. Documentation
does not make explicit explainations about which one to use, or when
to use one not the other, how good/bad reimplementing both can be, and
such.

I gave it a try and got a first solution in
https://bugs.webkit.org/attachment.cgi?id=41862 . Basically it

* extends QWebPageClient adding a pure virtual createWindow method.
* Implements createWindow in both QWebView and QGraphicsWebView
private classes, just calling its main classes' createWindow methods.
* Changes QWebPage::createWindow to call PageClient's createWindow.

So code'd look like:

QWebPage *QWebPage::createWindow(WebWindowType type)
{
  if (d->client)
    return d->client->createWindow(type);
  return 0;
}

Although it is simpler and more widget independent, it still has some issues:

0) previously a setView is required for QWebPage's createWindow to
calls its view's createWindow method, just to forward a "Page" back to
WebCore. Current patch makes setPage enough for it to work. Is it ok ?

1) Suppose I have one Q{Graphics}WebView instance and many QWebPage's,
just swapping them into the view (via setPage) as
in a TAB logic. With the approach proposed in the patch (see above),
the if-check "if (d->client)" would make only currently swapped
(viewed) qwebpage's
to get createWindow called. Is that too bad ?

Well, we could keep it as is, and explicit this behavior in the
documentation, saying that reimplementing this method is needed to
handle such cases.

2) it is not clear if we should still add a createWindow to
QGraphicsWebView. Again, if we do (and patch does) we should improve
the documentation to mention when/if/why using one createWindow
(qwebpage x q{graphics}webview) is preferable than other ?

I am open to suggestion ...

-- 
--Antonio Gomes


More information about the webkit-qt mailing list