Bug 25867 - [QT] Implement original_url getter method to the API
Summary: [QT] Implement original_url getter method to the API
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 27444
Blocks:
  Show dependency treegraph
 
Reported: 2009-05-19 10:29 PDT by Antonio Gomes
Modified: 2009-07-29 08:21 PDT (History)
4 users (show)

See Also:


Attachments
patch 0.1 - implement originalUrl support (2.65 KB, patch)
2009-07-06 11:21 PDT, Antonio Gomes
no flags Details | Formatted Diff | Diff
patch 0.2 - implement originalUrl support (2.69 KB, patch)
2009-07-06 12:51 PDT, Antonio Gomes
hausmann: review-
Details | Formatted Diff | Diff
patch 0.2 - implement originalUrl support (6.65 KB, patch)
2009-07-16 12:55 PDT, Antonio Gomes
no flags Details | Formatted Diff | Diff
patch 0.3 - implement originalUrl support (6.70 KB, patch)
2009-07-17 11:06 PDT, Antonio Gomes
no flags Details | Formatted Diff | Diff
patch 0.4 - implement originalUrl support (7.56 KB, patch)
2009-07-20 08:08 PDT, Antonio Gomes
no flags Details | Formatted Diff | Diff
patch 0.5 - implement originalUrl support (7.64 KB, patch)
2009-07-23 13:23 PDT, Antonio Gomes
no flags Details | Formatted Diff | Diff
patch 0.6 - implement originalUrl support (8.53 KB, patch)
2009-07-24 05:52 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-05-19 10:29:31 PDT
In some cases (e.g. sites that redirects) the application may be interested in the original URL entered by the user, not only in the resolved one.

We should consider including support for that in our API, if convenient.
Comment 1 Kenneth Rohde Christiansen 2009-05-19 10:32:01 PDT
Please name it originalUrl() and let it return a QUrl. What to do it they user gives an invalid URL? what will originalUrl return? a nulled QUrl?
Comment 2 Antonio Gomes 2009-06-03 06:09:30 PDT
(In reply to comment #1)
> What to do it they user gives an invalid URL? what will originalUrl return? a nulled QUrl?

it will return an empty qurl, not null.

Comment 3 Antonio Gomes 2009-06-03 06:09:59 PDT
TESTCASE:

In practical terms, I am calling "original url" here the url fixed but not yet resolved
loaded by the current main frame, e.g. :

1) if user runs: $ ./QtLauncher google.com
2) it gets "fixed" to "http://google.com/"
3) it gets "resolved" to "http://www.google.com"
4) sometimes it also gets "redirected" to other pages , e.g.
"http://www.google.com.br"

--> originalUrl() would return 2).
--> url() would return either 3) or 4).

SCENARIO:

The motivation for this simple add comes from a feature we needed to implement a while back while working on a frontend for our webkit-efl port [1], and we thought that it could also be useful for other ports.

[1] http://code.staikos.net/cgi-bin/gitweb.cgi?p=webkit;a=shortlog;h=shared/webkit-efl

In our case we had the following scenario:

1) we have a history management implemented in ui side.
2) during the mainFrame's loading process, it firstly calls |WebCoreSupport::updateGlobalHistory()| passing the URL being loaded(see quoted code below). It is the url not resolved or redirected yet.

(...)
void FrameLoaderClientQt::updateGlobalHistory()
{
   QWebHistoryInterface *history = QWebHistoryInterface::defaultInterface();
   if (history)
       history->addHistoryEntry(m_frame->loader()->documentLoader()->urlForHistory().prettyURL());
}
(...)

We stored the url passed in in our history database as a primary key.

3) yet in the loading process, server responses to the loading process and we get urlChanged and titleChanged signals emited. In our case, we want to update the history database entry of the just obtained |title| for the web page. At that time (when titleChanged signal gets emited) we get the original url by calling mainFrame->originalUrl(), find the history entry in the database
associated to it and update it w/ the new title value.
Comment 4 Antonio Gomes 2009-07-06 11:21:29 PDT
Created attachment 32309 [details]
patch 0.1 - implement originalUrl support

ps: I have not figure out a good way to add tests to this. any input welcome.
Comment 5 Antonio Gomes 2009-07-06 12:51:41 PDT
Created attachment 32310 [details]
patch 0.2 - implement originalUrl support

updated to ToT
Comment 6 Simon Hausmann 2009-07-14 07:16:55 PDT
Comment on attachment 32310 [details]
patch 0.2 - implement originalUrl support

Clearing from the review queue as per comment from IRC:
<tonikitoo> manyoso, tronical i am finishing originalURl patch ... adding a better test (as suggested by fawek) and better docs, as per simon's suggestion ...
Comment 7 Antonio Gomes 2009-07-16 12:55:58 PDT
Created attachment 32888 [details]
patch 0.2 - implement originalUrl support

Test added: it creates a proper FakeNetworkAccessManager (:QNetworkAccessManager) and FakeReply (:QNetworkReply) in order to a redirect.
Comment 8 Antonio Gomes 2009-07-16 12:59:18 PDT
Comment on attachment 32888 [details]
patch 0.2 - implement originalUrl support

see patch 0.3 also in

http://code.staikos.net/cgi-bin/gitweb.cgi?p=webkit;a=commitdiff;h=7aa68b24bcf949b3f78c25008e48a5de44d074b6
Comment 9 Antonio Gomes 2009-07-16 16:13:52 PDT
Comment on attachment 32888 [details]
patch 0.2 - implement originalUrl support

cleanring review flag ... improvements to come !
Comment 10 Antonio Gomes 2009-07-17 11:06:24 PDT
Created attachment 32958 [details]
patch 0.3 - implement originalUrl support

WIP: cleaned up
Comment 11 Antonio Gomes 2009-07-20 08:08:23 PDT
Created attachment 33085 [details]
patch 0.4 - implement originalUrl support
Comment 12 Antonio Gomes 2009-07-20 08:09:40 PDT
patch is ready for review, but can not land before patch in bug 27444.

setting it as a dep.
Comment 13 Antonio Gomes 2009-07-23 13:23:54 PDT
Created attachment 33361 [details]
patch 0.5 - implement originalUrl support

ready for review.
Comment 14 Antonio Gomes 2009-07-23 14:12:39 PDT
Comment on attachment 33361 [details]
patch 0.5 - implement originalUrl support

clearing review again. got comments from itc
Comment 15 Antonio Gomes 2009-07-24 05:52:38 PDT
Created attachment 33432 [details]
patch 0.6 - implement originalUrl support

1) ran style code script
2) left name as originalURL to match w/ QWebHistory::originalURL (as w/ gtk API )
3) updated to ToT
4) adde class doc overview
Comment 16 Antonio Gomes 2009-07-24 07:04:55 PDT
branch where to find the neswest patch (easier cherry pick)

http://code.staikos.net/cgi-bin/gitweb.cgi?p=webkit;a=shortlog;h=antonio/original-url
Comment 17 Simon Hausmann 2009-07-24 08:14:58 PDT
Comment on attachment 33432 [details]
patch 0.6 - implement originalUrl support

r=me

Please submit a separate patch for the function renaming, as discussed on IRC :)
Comment 18 Antonio Gomes 2009-07-24 11:22:29 PDT
fixed

thanks simon, adam and kenneth. 

(In reply to comment #17)
> (From update of attachment 33432 [details])
> r=me

landed in r46364

> Please submit a separate patch for the function renaming, as discussed on IRC
> :)

landed in r46367 (r=Adam Treat on irc)
Comment 19 Robert Hogan 2009-07-27 14:49:48 PDT
Why the check for d->frameLoaderClient->m_loadSucceeded when deciding which url to return?

This check happens to break my use-case of this feature, which is to handle SSL failures that occur as the result of redirects (e.g. https://hotmail.com). When the failure occurs m_loadSucceeded is not yet true, and won't be until the ssl failure is accepted by the user.
Comment 20 Antonio Gomes 2009-07-28 12:15:59 PDT
(In reply to comment #19)
> Why the check for d->frameLoaderClient->m_loadSucceeded when deciding which url
> to return?

mwenge, when the load is not successful (e.g. user loads an non-existent url) the d->loader()->activeDocumentLoader stays referring the latest successful load (not to the latest failing one). so we could not query the originalUrl off of it.

please see details in bug 27444 .

given that, I used this m_loadSucceeded flag to identify if the load was successful or not. If it is not, we do a special handler for it.

> This check happens to break my use-case of this feature, which is to handle SSL
> failures that occur as the result of redirects (e.g. https://hotmail.com). When
> the failure occurs m_loadSucceeded is not yet true, and won't be until the ssl
> failure is accepted by the user.

so it is not TRUE yet BUT load has not finished ... hum ... right I will think of something for it. we could have another bug for it, maybe
Comment 21 Antonio Gomes 2009-07-29 08:21:43 PDT
> > This check happens to break my use-case of this feature, which is to handle SSL
> > failures that occur as the result of redirects (e.g. https://hotmail.com). When
> > the failure occurs m_loadSucceeded is not yet true, and won't be until the ssl
> > failure is accepted by the user.
> 
> so it is not TRUE yet BUT load has not finished ... hum ... right I will think
> of something for it. we could have another bug for it, maybe

filed bug 27804