Bug 30213 - [Qt] Avoid ErrorPageExtensionOption to hold empty 'url' field
Summary: [Qt] Avoid ErrorPageExtensionOption to hold empty 'url' field
Status: RESOLVED WORKSFORME
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P3 Normal
Assignee: QtWebKit Unassigned
URL:
Keywords: Qt
Depends on:
Blocks: 30216
  Show dependency treegraph
 
Reported: 2009-10-08 08:36 PDT by Antonio Gomes
Modified: 2010-03-23 08:34 PDT (History)
3 users (show)

See Also:


Attachments
patch 0.1 (2.11 KB, patch)
2009-10-08 08:45 PDT, Antonio Gomes
hausmann: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antonio Gomes 2009-10-08 08:36:27 PDT
In some cases, QNetworkReplyHandler's m_reply holds an empty url.

m_reply->url(); <--- empty.

it happend for example in some non-successful page loads. e.g. if one do:

<quote>
frame>setUrl("http://any.thing.invalid.here/");
<quote/>

When it happens, the ResourceError passed up to FrameLoadClientQt by QNetworkReplyHandle::finish (via MainResourceLoader -> DocumentLoader -> FrameLoader) will fill ErrorPageExtensionOption's url field with an empty url.

it might not be a helpful situation for client application handling error pages ...
Comment 1 Antonio Gomes 2009-10-08 08:45:32 PDT
Created attachment 40875 [details]
patch 0.1
Comment 2 Simon Hausmann 2009-10-09 07:36:54 PDT
Comment on attachment 40875 [details]
patch 0.1


> -        QUrl url = m_reply->url();
> +        QUrl url = (m_reply->url().toString().isNull()) ? m_request.url(): m_reply->url();

Instead of converting the url to a QString first before checking if it's null, can you call isEmpty() on QUrl instead?

I.e.

QUrl url = m_reply->url();
if (url.isEmpty())
    url = m_request.url();


Is there any way to write a unit test for this? :)
Comment 3 Antonio Gomes 2009-10-11 21:17:07 PDT
thx for your comments, simon.

> Instead of converting the url to a QString first before checking if it's null,
> can you call isEmpty() on QUrl instead?

sure thing.

> I.e.
> 
> QUrl url = m_reply->url();
> if (url.isEmpty())
>     url = m_request.url();

ok

> Is there any way to write a unit test for this? :)

i will try to get a auto test for it, before a next iteration here ...
Comment 4 Antonio Gomes 2010-03-23 08:34:43 PDT
never mind. I just tried it again, and it works fine now