[webkit-reviews] review denied: [Bug 54380] [Qt]tst_QWebPage::modified() asserts : [Attachment 83098] Patch for review

[bugzilla-daemon at webkit.org]

Andreas Kling <kling at webkit.org> has denied Aparna Nandyal
<aparna.nand at wipro.com>'s request for review:
Bug 54380: [Qt]tst_QWebPage::modified() asserts
https://bugs.webkit.org/show_bug.cgi?id=54380

Attachment 83098: Patch for review
https://bugs.webkit.org/attachment.cgi?id=83098&action=review

------- Additional Comments from Andreas Kling <kling at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=83098&action=review

r- because we should implement QWebFrame::setUrl() in a cleaner way.

> Source/WebKit/qt/Api/qwebframe.cpp:765
> +    // Resetting the load type here can be removed, if resetting
> +    // happens at end of every load or abort
> +    d->frame->loader()->resetLoadType();
>      d->frame->loader()->activeDocumentLoader()->writer()->begin(absolute);
>      d->frame->loader()->activeDocumentLoader()->writer()->end();

I have a feeling that we're doing something wrong here (way too many ->'s..)

The difference between QWebFrame::setUrl() and QWebFrame::load() is that
setUrl() should clear the frame immediately, instead of when the new page has
content. There must be a cleaner way than reaching all the way down to the
writer(). FrameLoader::clear() perhaps?