Bug 75445 – [Qt][WK2] QQuickWebView breaks when an empty url is loaded (debug mode)

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Qt

[Rafael Brandao]

I've tested it on Chrome and Safari, and both of them just ignore the request (once it is an empty url) and stays in the same page. If we don't handle this and request to load an empty url, an empty ResourceRequest is created, asserting on MainResourceLoader::willSendRequest. This behavior is well handled when we are using substitute data with an empty url as base url: the base url is changed to "about:blank", thus not crashing at all. It makes sense to ignore empty url requests, but it is weird that we crash when we get it.

Right now, QQuickWebView starts with an empty url and no content displayed. Once we load something with it, we can never return to the original state: no url and no content (this is how I've found this bug). This use case is very appropriate to create test cases, but I can't think of anything else where this would fit well.

A possible solution is to make QQuickWebView start with the url "about:blank" instead of a blank url, and ignore loads for empty urls as other browsers do (and add this to the documentation).

We could also change the way WebCore works handling it there, but I still couldn't find out why that assert is there and I suppose there's a good reason.

What you guys think?

[Rafael Brandao]

Ignoring the assert there, the url is loaded just like it was expected... it loads an empty page. The code on MainResourceLoader is very old, it is possible that the assert is no longer necessary but I'm still looking on it.

The comment preceding the assert on MainResourceLoader::willSendRequest is very confusing, it says the following code doesn't have any asserts for example. :-/

[Rafael Brandao]

Created attachment 120992 [details]
Patch

[Rafael Brandao]

(In reply to comment #2)
> Created an attachment (id=120992) [details]
> Patch

As we've discussed on IRC, it make sense to ignore an empty url when it is requested to be loaded.

[Kenneth Rohde Christiansen]

Comment on attachment 120992 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=120992&action=review

> Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:757
>  void QQuickWebView::load(const QUrl& url)
>  {
> +    if (url.isEmpty())
> +        return;
> +
>      Q_D(QQuickWebView);
>      d->webPageProxy->loadURL(url.toString());
>  }

Should we emit load failed? a user might call load while doing another load which say reached 50% and with this change I believe it will just continue loading that page. Is this intended and can such a case be tested?

[Rafael Brandao]

(In reply to comment #4)
> (From update of attachment 120992 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=120992&action=review
> 
> > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:757
> >  void QQuickWebView::load(const QUrl& url)
> >  {
> > +    if (url.isEmpty())
> > +        return;
> > +
> >      Q_D(QQuickWebView);
> >      d->webPageProxy->loadURL(url.toString());
> >  }
> 
> Should we emit load failed? a user might call load while doing another load which say reached 50% and with this change I believe it will just continue loading that page. Is this intended and can such a case be tested?

Maybe load fail would look like we actually tried to load it but we are just ignoring. I've looked how Chrome behaves and it keeps the old page loading, like nothing has happened (as we're doing). Safari reloads the same page when you use an empty url.

[Kenneth Rohde Christiansen]

anyway a test for that is needed.

[Rafael Brandao]

Created attachment 121103 [details]
Patch

[WebKit Review Bot]

Comment on attachment 121103 [details]
Patch

Clearing flags on attachment: 121103

Committed r104287: <http://trac.webkit.org/changeset/104287>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.