82506 2012-03-28 12:33:42 -0700 [Qt][WK2] QQuickWebView fails to load an url if the same url is requested again to be loaded 2012-10-01 09:10:32 -0700 1 1 1 Unclassified WebKit WebKit Qt 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 76773 1 rafael.lobo michael.bruning abecsi cmarcelo hausmann jesus marcelo.lira menard michael.bruning vestbo webkit.review.bot zoltan oldest_to_newest 590284 0 rafael.lobo 2012-03-28 12:33:42 -0700 If you create a webview dinamically passing the initial url to it and then, before it completes the loading, you change the url to the same url, then the loading will fail giving a 404 error. It prints out this: QHttpNetworkConnectionPrivate::_q_hostLookupFinished could not dequeu request QNetworkReplyImplPrivate::error: Internal problem, this method must only be called once. 590306 1 marcelo.lira 2012-03-28 12:52:42 -0700 It's not exactly a 404, it is just that WebView's LoadFailedStatus is set because the first loading was interrupted by the second. Although I agree that if the second url is the same as the first it should be smarter and do nothing instead of complaining. 590314 2 rafael.lobo 2012-03-28 12:56:16 -0700 We can close this if this is what is expected. Hey Simon, what do you think? 590371 3 marcelo.lira 2012-03-28 13:44:41 -0700 I was thinking that if the user interrupted the first page load caused by attributing something to webView.url, and then setting again webView.url with the same or other address, the first page load should not be considered a failure, because the user did this on purpose. In other words, doing: webView.url = "www.one.com" webView.url = "www.one.com" should be semantically equivalent to: webView.load("www.one.com") webView.reload() and webView.load("www.one.com") webView.stop() webView.load("www.two.com") for when the addresses are different. It should not issue an error in both cases, because the first page was expected to stop loading. 593194 4 hausmann 2012-04-01 23:23:54 -0700 (In reply to comment #2) > We can close this if this is what is expected. Hey Simon, what do you think? Perhaps I misunderstood this, but IMHO conceptually calling view->setUrl(view->url()) should be a noop and not cause any errors. In other words: setting a property to the value that's already set should not do anything. Does that make sense? 593787 5 rafael.lobo 2012-04-02 13:44:40 -0700 (In reply to comment #4) > (In reply to comment #2) > > We can close this if this is what is expected. Hey Simon, what do you think? > > Perhaps I misunderstood this, but IMHO conceptually calling > > view->setUrl(view->url()) > > should be a noop and not cause any errors. In other words: setting a property to the value that's already set should not do anything. > > Does that make sense? That makes perfect sense. I believe when user really wants to trigger a reload, then he should use the reload function, instead of view.url = view.url. I didn't look at the code, but the problem might lie around the fact that the current url value of a web view is not the url that is being loaded, but the last url completely loaded, right Marcelo? 594261 6 hausmann 2012-04-03 00:51:18 -0700 (In reply to comment #5) > [...] I believe when user really wants to trigger a reload, then he should use the reload function, instead of view.url = view.url. Exactly! 595745 7 marcelo.lira 2012-04-04 11:25:43 -0700 (In reply to comment #5) > (In reply to comment #4) > > (In reply to comment #2) > > > We can close this if this is what is expected. Hey Simon, what do you think? > > > > Perhaps I misunderstood this, but IMHO conceptually calling > > > > view->setUrl(view->url()) > > > > should be a noop and not cause any errors. In other words: setting a property to the value that's already set should not do anything. > > > > Does that make sense? > > That makes perfect sense. I believe when user really wants to trigger a reload, then he should use the reload function, instead of view.url = view.url. > > I didn't look at the code, but the problem might lie around the fact that the current url value of a web view is not the url that is being loaded, but the last url completely loaded, right Marcelo? (In reply to comment #5) > (In reply to comment #4) > > (In reply to comment #2) > > > We can close this if this is what is expected. Hey Simon, what do you think? > > > > Perhaps I misunderstood this, but IMHO conceptually calling > > > > view->setUrl(view->url()) > > > > should be a noop and not cause any errors. In other words: setting a property to the value that's already set should not do anything. > > > > Does that make sense? > > That makes perfect sense. I believe when user really wants to trigger a reload, then he should use the reload function, instead of view.url = view.url. > > I didn't look at the code, but the problem might lie around the fact that the current url value of a web view is not the url that is being loaded, but the last url completely loaded, right Marcelo? Yes, QQuickWebView::url() returns the url from webPageProxy->mainFrame(), but the frame may not be ready when WebView.url is called, so in QML it will result in an empty string. 595749 8 marcelo.lira 2012-04-04 11:35:29 -0700 (In reply to comment #6) > (In reply to comment #5) > > [...] I believe when user really wants to trigger a reload, then he should use the reload function, instead of view.url = view.url. > > Exactly! I agree that the nop is the behavior that makes sense. In any case, the problem remains that there will be times when QQuickWebView will not know which url it is loading, because it calls webPageProxy->loadURL(url) and it has to wait for the readying of the frame before being able to ask the url again. (I mean, it may ask I think the solution to this is to store the url also in the QQuickWebView (and update it when there are redirections, etc) so that we may compare it to new requests, and also always answer when QQuickWebView::url() is called. 595752 9 marcelo.lira 2012-04-04 11:38:29 -0700 (In reply to comment #8) > (In reply to comment #6) > > (In reply to comment #5) > > > [...] I believe when user really wants to trigger a reload, then he should use the reload function, instead of view.url = view.url. > > > > Exactly! > I agree that the nop is the behavior that makes sense. > In any case, the problem remains that there will be times when QQuickWebView will not know which url it is loading, because it calls webPageProxy->loadURL(url) and it has to wait for the readying of the frame before being able to ask the url again. (I mean, it may ask > I think the solution to this is to store the url also in the QQuickWebView (and update it when there are redirections, etc) so that we may compare it to new requests, and also always answer when QQuickWebView::url() is called. From the point of view of my last comment, this one is related to bug 77554. 656410 10 hausmann 2012-06-24 23:35:44 -0700 (In reply to comment #9) > (In reply to comment #8) > > (In reply to comment #6) > > > (In reply to comment #5) > > > > [...] I believe when user really wants to trigger a reload, then he should use the reload function, instead of view.url = view.url. > > > > > > Exactly! > > I agree that the nop is the behavior that makes sense. > > In any case, the problem remains that there will be times when QQuickWebView will not know which url it is loading, because it calls webPageProxy->loadURL(url) and it has to wait for the readying of the frame before being able to ask the url again. (I mean, it may ask > > I think the solution to this is to store the url also in the QQuickWebView (and update it when there are redirections, etc) so that we may compare it to new requests, and also always answer when QQuickWebView::url() is called. > > From the point of view of my last comment, this one is related to bug 77554. Indeed, and now that 77554 is fixed, how do we move forward with this issue? Is setUrl just missing the url == m_currentUrl shortcut? 728598 11 165811 michael.bruning 2012-09-26 08:35:46 -0700 Created attachment 165811 Patch 728639 12 165811 vestbo 2012-09-26 09:11:49 -0700 Comment on attachment 165811 Patch I think we need to keep the current behaviour. Loading the same url again, and reloading, are not the same. For example, reloading will restore the viewport position. 728661 13 michael.bruning 2012-09-26 09:28:06 -0700 (In reply to comment #12) > (From update of attachment 165811 [details]) > I think we need to keep the current behaviour. Loading the same url again, and reloading, are not the same. For example, reloading will restore the viewport position. Okay, true :). Should we close this bug then? 731733 14 rafael.lobo 2012-10-01 09:06:22 -0700 (In reply to comment #12) > (From update of attachment 165811 [details]) > I think we need to keep the current behaviour. Loading the same url again, and reloading, are not the same. For example, reloading will restore the viewport position. What is the current behavior? Do we still get an error when we pass the same url again? 731735 15 michael.bruning 2012-10-01 09:10:32 -0700 (In reply to comment #14) > (In reply to comment #12) > > (From update of attachment 165811 [details] [details]) > > I think we need to keep the current behaviour. Loading the same url again, and reloading, are not the same. For example, reloading will restore the viewport position. > > What is the current behavior? Do we still get an error when we pass the same url again? As discussed on irc, passing the same url again does not trigger an error, but basically results in a NOP. Closing this now, please holler if you believe it should be left open. 165811 2012-09-26 08:35:46 -0700 2012-09-26 09:11:49 -0700 Patch bug-82506-20120926173504.patch text/plain 4967 michael.bruning U3VidmVyc2lvbiBSZXZpc2lvbjogMTI5NjMxCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggNTQ5MmRjZTlhMTAyYjc0 OGYzNzdhZDQ0Njk3MDM1OGQ3OWFkZWYxZS4uOGIwMTBkNWVlNmMwNzc3MTk2ZGMzOTQxYTIzZDBh OGM1OWQ5ZWZmYSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIxIEBACisyMDEyLTA5LTI2ICBNaWNo YWVsIEJyw7xuaW5nICA8bWljaGFlbC5icnVuaW5nQGRpZ2lhLmNvbT4KKworICAgICAgICBbUXRd W1dLMl0gUVF1aWNrV2ViVmlldyBmYWlscyB0byBsb2FkIGFuIHVybCBpZiB0aGUgc2FtZSB1cmwg aXMgcmVxdWVzdGVkIGFnYWluIHRvIGJlIGxvYWRlZAorICAgICAgICBodHRwczovL2J1Z3Mud2Vi a2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9ODI1MDYKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JP RFkgKE9PUFMhKS4KKworICAgICAgICBDaGFuZ2VzIHRoZSBiZWhhdmlvciBvZiBzZXRVcmwgc28g dGhhdCBpdCBubyBsb25nZXIgbG9hZHMgYSB1cmwgdGhhdCBpcyBwYXNzZWQgYnkgdGhlIGNhbGxl cgorICAgICAgICBpZiBpcyBlcXVhbCB0byB0aGUgdXJsIHRoYXQgaXMgY3VycmVudGx5IGxvYWRl ZC4KKworICAgICAgICBBbHNvIGFkZHMgYSB0ZXN0IHRvIHZlcmlmeSB0aGUgdXJsIGlzIG5vdCBy ZWxvYWRlZC4KKworICAgICAgICAqIFVJUHJvY2Vzcy9BUEkvcXQvcXF1aWNrd2Vidmlldy5jcHA6 CisgICAgICAgIChRUXVpY2tXZWJWaWV3OjpzZXRVcmwpOgorICAgICAgICAqIFVJUHJvY2Vzcy9B UEkvcXQvdGVzdHMvcXF1aWNrd2Vidmlldy90c3RfcXF1aWNrd2Vidmlldy5jcHA6CisgICAgICAg ICh0c3RfUVF1aWNrV2ViVmlldyk6CisgICAgICAgICh0c3RfUVF1aWNrV2ViVmlldzo6ZG9Ob3RS ZWxvYWRDdXJyZW50VXJsRnJvbVNldFVybCk6IEFkZGVkLgorCiAyMDEyLTA5LTI1ICBSYXBoYWVs IEt1Ym8gZGEgQ29zdGEgIDxyYXBoYWVsLmt1Ym8uZGEuY29zdGFAaW50ZWwuY29tPgogCiAgICAg ICAgIFtEUlRdW1dUUl0gU3VwcG9ydCBvdmVycmlkaW5nIHRoZSAnV2ViS2l0RGlzcGxheUltYWdl c0tleScgcHJlZmVyZW5jZQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL0FQ SS9xdC9xcXVpY2t3ZWJ2aWV3LmNwcCBiL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9BUEkvcXQv cXF1aWNrd2Vidmlldy5jcHAKaW5kZXggYjc4Y2FlMGYzZWRiOTEyYmVmMDJjMTdmM2E2MmU4YWQ4 NjVlNTZkMC4uYWQ5ZDM2MGIxNDE3NzFjNzhlNjcxMThhODdmODliZWJmMTc0M2Q5YyAxMDA2NDQK LS0tIGEvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL0FQSS9xdC9xcXVpY2t3ZWJ2aWV3LmNwcAor KysgYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvQVBJL3F0L3FxdWlja3dlYnZpZXcuY3BwCkBA IC0xNTgzLDYgKzE1ODMsOSBAQCB2b2lkIFFRdWlja1dlYlZpZXc6OnNldFVybChjb25zdCBRVXJs JiB1cmwpCiAgICAgaWYgKHVybC5pc0VtcHR5KCkpCiAgICAgICAgIHJldHVybjsKIAorICAgIGlm ICh1cmwgPT0gZC0+bV9jdXJyZW50VXJsKQorICAgICAgICByZXR1cm47CisKICAgICBkLT53ZWJQ YWdlUHJveHktPmxvYWRVUkwodXJsLnRvU3RyaW5nKCkpOwogICAgIGVtaXRVcmxDaGFuZ2VJZk5l ZWRlZCgpOwogfQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL0FQSS9xdC90 ZXN0cy9xcXVpY2t3ZWJ2aWV3L3RzdF9xcXVpY2t3ZWJ2aWV3LmNwcCBiL1NvdXJjZS9XZWJLaXQy L1VJUHJvY2Vzcy9BUEkvcXQvdGVzdHMvcXF1aWNrd2Vidmlldy90c3RfcXF1aWNrd2Vidmlldy5j cHAKaW5kZXggNWI5OGFlN2I3NDhiMzcxNmY2OGRjYTIxMzUxM2VjY2IzODcwMDA4Yi4uNWIxNTRl ZDIxZmU1YzRiNjM2ZTlkN2VjOWFjZTFhMzUxNmUyZTVmNiAxMDA2NDQKLS0tIGEvU291cmNlL1dl YktpdDIvVUlQcm9jZXNzL0FQSS9xdC90ZXN0cy9xcXVpY2t3ZWJ2aWV3L3RzdF9xcXVpY2t3ZWJ2 aWV3LmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvQVBJL3F0L3Rlc3RzL3FxdWlj a3dlYnZpZXcvdHN0X3FxdWlja3dlYnZpZXcuY3BwCkBAIC00Nyw2ICs0Nyw3IEBAIHByaXZhdGUg UV9TTE9UUzoKICAgICB2b2lkIHJlbG9hZCgpOwogICAgIHZvaWQgc3RvcCgpOwogICAgIHZvaWQg bG9hZFByb2dyZXNzKCk7CisgICAgdm9pZCBkb05vdFJlbG9hZEN1cnJlbnRVcmxGcm9tU2V0VXJs KCk7CiAgICAgdm9pZCBzY3JvbGxSZXF1ZXN0KCk7CiAKICAgICB2b2lkIHNob3coKTsKQEAgLTI0 OCw2ICsyNDksMTkgQEAgdm9pZCB0c3RfUVF1aWNrV2ViVmlldzo6bG9hZFByb2dyZXNzKCkKICAg ICBRQ09NUEFSRSh3ZWJWaWV3KCktPmxvYWRQcm9ncmVzcygpLCAxMDApOwogfQogCit2b2lkIHRz dF9RUXVpY2tXZWJWaWV3Ojpkb05vdFJlbG9hZEN1cnJlbnRVcmxGcm9tU2V0VXJsKCkKK3sKKyAg ICB3ZWJWaWV3KCktPnNldFVybChRVXJsOjpmcm9tTG9jYWxGaWxlKFFMYXRpbjFTdHJpbmcoVEVT VFNfU09VUkNFX0RJUiAiL2h0bWwvYmFzaWNfcGFnZS5odG1sIikpKTsKKyAgICBRVkVSSUZZKHdh aXRGb3JMb2FkU3VjY2VlZGVkKHdlYlZpZXcoKSkpOworCisgICAgUUNPTVBBUkUod2ViVmlldygp LT51cmwoKS5wYXRoKCksIFFMYXRpbjFTdHJpbmcoVEVTVFNfU09VUkNFX0RJUiAiL2h0bWwvYmFz aWNfcGFnZS5odG1sIikpOworCisgICAgUVNpZ25hbFNweSBsb2FkaW5nQ2hhbmdlZFNweSh3ZWJW aWV3KCksIFNJR05BTChsb2FkaW5nQ2hhbmdlZChRV2ViTG9hZFJlcXVlc3QqKSkpOworICAgIHdl YlZpZXcoKS0+c2V0VXJsKFFVcmw6OmZyb21Mb2NhbEZpbGUoUUxhdGluMVN0cmluZyhURVNUU19T T1VSQ0VfRElSICIvaHRtbC9iYXNpY19wYWdlLmh0bWwiKSkpOworICAgIFFWRVJJRlkoIXdhaXRG b3JTaWduYWwod2ViVmlldygpLCBTSUdOQUwobG9hZGluZ0NoYW5nZWQoUVdlYkxvYWRSZXF1ZXN0 KikpLCAxMDAwKSk7CisgICAgUVZFUklGWSghbG9hZGluZ0NoYW5nZWRTcHkuY291bnQoKSk7Cit9 CisKIHZvaWQgdHN0X1FRdWlja1dlYlZpZXc6OnNob3coKQogewogICAgIC8vIFRoaXMgc2hvdWxk IG5vdCBjcmFzaC4KZGlmZiAtLWdpdCBhL1Rvb2xzL0NoYW5nZUxvZyBiL1Rvb2xzL0NoYW5nZUxv ZwppbmRleCA0ZGE3NDRkZTA3OTVjMzIyOTBkNjYwYTBjMzZjYTM2NjdkNGEwMmZhLi5hOGJmNDJi ZjQ5NmRmYjVlMDc3YzE3Yjc0NDI0ZWVlN2QwNTJhYzFjIDEwMDY0NAotLS0gYS9Ub29scy9DaGFu Z2VMb2cKKysrIGIvVG9vbHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTItMDktMjYg IE1pY2hhZWwgQnLDvG5pbmcgIDxtaWNoYWVsLmJydW5pbmdAZGlnaWEuY29tPgorCisgICAgICAg IFtRdF1bV0syXSBRUXVpY2tXZWJWaWV3IGZhaWxzIHRvIGxvYWQgYW4gdXJsIGlmIHRoZSBzYW1l IHVybCBpcyByZXF1ZXN0ZWQgYWdhaW4gdG8gYmUgbG9hZGVkCisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD04MjUwNgorCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEFkZHMgZnVuY3Rpb25hbGl0eSB0byByZWxvYWQg dGhlIHVybCBpZiBlbnRlciBoYXMgYmVlbiBwcmVzc2VkIGluIHRoZSB1cmwgYmFyIGFuZCB0aGUg dXJsIAorICAgICAgICByZW1haW5zIHVuY2hhbmdlZC4gVGhpcyBpcyBuZWVkZWQgYmVjYXVzZSBR UXVpY2tXZWJWaWV3IG5vIGxvbmdlciByZWxvYWRzIGEgdXJsIHRoYXQgaXMKKyAgICAgICAgcGFz c2VkIHRocm91Z2ggdGhlIHVybCBwcm9wZXJ0eSBpZiB0aGF0IHNhbWUgdXJsIGlzIHRoZSBjdXJy ZW50IHVybC4gSG93ZXZlciwgbW9zdCBicm93c2VycworICAgICAgICByZWxvYWQgYSB1cmwgb24g ZW50ZXIga2V5IHByZXNzLCBldmVuIGlmIGl0IGhhcyBub3QgYmVlbiBjaGFuZ2VkLgorCisgICAg ICAgICogTWluaUJyb3dzZXIvcXQvcW1sL0Jyb3dzZXJXaW5kb3cucW1sOgorCiAyMDEyLTA5LTI2 ICBEaW51IEphY29iICA8ZGludS5zLmphY29iQGludGVsLmNvbT4KIAogICAgICAgICBVbnJldmll d2VkLiBBZGRpbmcgbXlzZWxmIGFzIGEgY29tbWl0dGVyLgpkaWZmIC0tZ2l0IGEvVG9vbHMvTWlu aUJyb3dzZXIvcXQvcW1sL0Jyb3dzZXJXaW5kb3cucW1sIGIvVG9vbHMvTWluaUJyb3dzZXIvcXQv cW1sL0Jyb3dzZXJXaW5kb3cucW1sCmluZGV4IGM3MzMwOGUxNWI4YWFjODA2OTRkNDAxYzg2YzYy YTBhMjAzM2Y5OWUuLmEwODNiMDEwOWUyOWFkODM3ZDYxNzkwZmIyZmJkZDcyY2UxZTA4N2QgMTAw NjQ0Ci0tLSBhL1Rvb2xzL01pbmlCcm93c2VyL3F0L3FtbC9Ccm93c2VyV2luZG93LnFtbAorKysg Yi9Ub29scy9NaW5pQnJvd3Nlci9xdC9xbWwvQnJvd3NlcldpbmRvdy5xbWwKQEAgLTMxMiw3ICsz MTIsMTIgQEAgUmVjdGFuZ2xlIHsKIAogICAgICAgICAgICAgICAgIEtleXMub25SZXR1cm5QcmVz c2VkOnsKICAgICAgICAgICAgICAgICAgICAgY29uc29sZS5sb2coIk5hdmlnYXRpbmcgdG86ICIs IGFkZHJlc3NMaW5lLnRleHQpCi0gICAgICAgICAgICAgICAgICAgIGxvYWQodXRpbHMudXJsRnJv bVVzZXJJbnB1dChhZGRyZXNzTGluZS50ZXh0KSkKKyAgICAgICAgICAgICAgICAgICAgdmFyIGVu dGVyZWRVcmwgPSB1dGlscy51cmxGcm9tVXNlcklucHV0KGFkZHJlc3NMaW5lLnRleHQpCisKKyAg ICAgICAgICAgICAgICAgICAgaWYgKGVudGVyZWRVcmwgPT0gd2ViVmlldy51cmwpCisgICAgICAg ICAgICAgICAgICAgICAgICByZWxvYWQoKQorICAgICAgICAgICAgICAgICAgICBlbHNlCisgICAg ICAgICAgICAgICAgICAgICAgICBsb2FkKHV0aWxzLnVybEZyb21Vc2VySW5wdXQoYWRkcmVzc0xp bmUudGV4dCkpCiAgICAgICAgICAgICAgICAgfQogCiAgICAgICAgICAgICAgICAgcHJvcGVydHkg dXJsIHVybAo=