window.open() from js opens a new window but clicking on <a target="_blank" href="#">..</a> doesn't do anything. I will attach a patch ASAP.
Created attachment 21163 [details] Show the window containing the newly created QWebPage Can newPage->view() be null in this case?
Comment on attachment 21163 [details] Show the window containing the newly created QWebPage I don't think this is the right place to call show(). dispatchCreatePage() is called from FrameLoader::continueAfterNewWindowPolicy, which shortly afterwards calls FrameLoaderClient::dispatchShow(). That function is currently not implemented and I believe that's where a show() call should go.
Oh, right. I didn't know of dispatchShow. I will update the patch as soon as possible.
If the browser wants to open the new page in a tab instead of using a separate window then showing the toplevel windows is not the best idea. How about adding a virtual method QWebPage::showWindow() with a default implentation that shows the toplevel window? I proposed something similar also for the GTK port, see bug #19143.
I agree about not calling show() on the QWidget directly. Unfortunately we cannot add virtual functions to QWebPage as it breaks binary compatibility. Fortunately a more consistent API would be to add a signal, similar to menuBarVisibilityChangeRequested or statusBarVisibilityChangeRequested. By default QWebView could connect the signal to its own private slot to call QWidget::show()/hide() accordingly. This way the implementation of ChromeClientQt::show() and FrameLoaderClientQt::dispatchShow() can both be changed to emit the new signal. (In the windows port they also both map to the same call)
Hi, I tried the issue with this test case on the trunk: <html> <body> <a target="_blank" href="#">blank</a> <a onClick="window.open()">blankJS</a> </body> </html> in both cases a new window was created. It seems that the problem is resolved. I'm not assigned to the bug, so if I missed something feel free to reopen it.