Summary: | [Qt][WK2] QQuickWebView breaks when an empty url is loaded (debug mode) | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Rafael Brandao <rafael.lobo> | ||||||
Component: | WebKit Qt | Assignee: | Rafael Brandao <rafael.lobo> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Critical | CC: | cmarcelo, hausmann, jesus, kenneth, menard, noam, webkit.review.bot, zoltan | ||||||
Priority: | P2 | Keywords: | Qt, QtTriaged | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Rafael Brandao
2012-01-02 13:35:03 PST
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. :-/ Created attachment 120992 [details]
Patch
(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. 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? (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. anyway a test for that is needed. Created attachment 121103 [details]
Patch
Comment on attachment 121103 [details] Patch Clearing flags on attachment: 121103 Committed r104287: <http://trac.webkit.org/changeset/104287> All reviewed patches have been landed. Closing bug. |