[Webkit-unassigned] [Bug 50550] [Qt] QWebPage::createWindow does not recieve the proper window type...

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Dec 7 04:11:50 PST 2010


https://bugs.webkit.org/show_bug.cgi?id=50550





--- Comment #7 from Benjamin Poulain <benjamin.poulain at nokia.com>  2010-12-07 04:11:50 PST ---
(From update of attachment 75780)
View in context: https://bugs.webkit.org/attachment.cgi?id=75780&action=review

>> WebCore/ChangeLog:8
>> +        No new tests. (OOPS!)
> 
> You have an auto test. Either delete this line OR mention why it can not be tested.

I am in favor of extending the controller to have a layout test. It is too important to be limited to Qt autotests only.

> WebCore/page/WindowFeatures.cpp:159
> +    else if (keyString == "dialog")
> +        dialog = value;

In FrameLoader, there is an assert against this:
   ASSERT(!features.dialog || request.frameName().isEmpty());
This could never been hit before, could you check if it is still the case?

> WebKit/qt/tests/qwebpage/tst_qwebpage.cpp:359
> +    virtual QWebPage* createWindow(WebWindowType type)
> +    {
>          QWebPage* page = new TestPage(this);
>          createdWindows.append(page);
> +        windowType = type;
>          return page;

This is missleading in my opinion. You create a new page, but set the windowType of the current page, not the page returned by the call.

> WebKit/qt/tests/qwebpage/tst_qwebpage.cpp:@
>  void tst_QWebPage::acceptNavigationRequestWithNewWindow()

I would prefer a new test.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list