Bug 14807 – [gtk] API implementation: url and title

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Christian Dywan]

Currently the signal "title-changed" is emitted when either the url or the title has changed. Accordingly title and uri are provided as gchar* arguments. I suggest two changes.

1. For one we should split the signal into two separate signals, like "uri-changed" and "title-changed". This makes a lot more sense to me.

2. Further more we should add a webkit_gtk_page_get_title and a webkit_gtk_page_get_uri, since we do not currently have any means of retrieving either the uri or the title directly.

[Adam Roben (:aroben)]

(In reply to comment #0)
> 2. Further more we should add a webkit_gtk_page_get_title and a
> webkit_gtk_page_get_uri, since we do not currently have any means of retrieving
> either the uri or the title directly.

I think this model might be too simple. The Mac/Windows APIs expose WebURLRequest and WebDataSource classes which can be queried for their URL/title, respectively. This is needed because a WebView can have more than one of each of these associated with it at a time. -[WebView provisionalDataSource] and -[WebView dataSource] expose WebDataSource's that represent the current and completed load, respectively. This is very useful when creating a window title like "Loading -- Google", for example.

I think there are many benefits to mimicking the Mac/Windows API here.

[Adam Roben (:aroben)]

(In reply to comment #1)
> -[WebView provisionalDataSource] and -[WebView dataSource]

Whoops. Make that -[WebFrame provisionalDataSource] and -[WebFrame dataSource]

[Xan Lopez]

(In reply to comment #1)
> I think this model might be too simple. The Mac/Windows APIs expose
> WebURLRequest and WebDataSource classes which can be queried for their
> URL/title, respectively. This is needed because a WebView can have more than
> one of each of these associated with it at a time. -[WebView
> provisionalDataSource] and -[WebView dataSource] expose WebDataSource's that
> represent the current and completed load, respectively. This is very useful
> when creating a window title like "Loading -- Google", for example.
> 
> I think there are many benefits to mimicking the Mac/Windows API here.

Are you simply suggesting to store the current and the "in-progress" title/url, or anything more sophisticated? Showing a temporary title like "Loading -- Google" and reverting to the old one (and old URL) in case of failure/cancellation would be certainly a nice touch.

[Adam Roben (:aroben)]

(In reply to comment #3)
> (In reply to comment #1)
> Are you simply suggesting to store the current and the "in-progress" title/url,
> or anything more sophisticated? Showing a temporary title like "Loading --
> Google" and reverting to the old one (and old URL) in case of
> failure/cancellation would be certainly a nice touch.

I'm suggesting that you consider following the WebDataSource/WebURLRequest model of the Mac/Win APIs. There are many uses for these objects beyond just getting the title/URL.

[Alp Toker]

(In reply to comment #4)
> I'm suggesting that you consider following the WebDataSource/WebURLRequest
> model of the Mac/Win APIs. There are many uses for these objects beyond just
> getting the title/URL.
> 

So, to get things up to date:

We do have a simple equivalent to WebURLRequest now.

We still sorely lack a WebDataSource.

We could do with convenience web_view_get_title() and web_view_get_uri() functions. Or maybe web_view_get_main_title() and web_view_get_main_uri() would be more accurate. The Mac API has such convenience accessors now.

[Christian Dywan]

Created attachment 27003 [details]
Implement title and uri on the view

Our API is still inconsistent in such simple cases. The frame has URI and title accessors and properties, while the view has not. That leads to clumsy calls to webkit_web_view_get_main_frame where anything like a data source would be overkill. On top of that there are no property notifications at all, so the frame properties are even only half usable.

I suggest that we fix the frame properties and implement the very same properties on the view. Most of the webkit applications will find this enough for all they ever need. The attached patch does that.

Come to think of it, what about making URI writable? It's pure convenience but would allow for neat tricks like applying an initial location in g_object_new or loading a page inside a graphical interface builder.

[Christian Dywan]

Created attachment 27024 [details]
Implement title and uri on the view #2

Updated. As suggested in bug 20306, this patch includes updating the URI also on navigation inside the same page. Plus it deprecates the "title-changed" signal which is now unneeded.

[Gustavo Noronha (kov)]

(In reply to comment #7)
> Created an attachment (id=27024) [review]
> Implement title and uri on the view #2
> 
> Updated. As suggested in bug 20306, this patch includes updating the URI also
> on navigation inside the same page. Plus it deprecates the "title-changed"
> signal which is now unneeded.
> 

I like the concept of using properties to notify of changes such as those. One thing that hit me while reading through the patch was that we might want to add some kind of DISABLE_DEPRECATED macro, and wrap signals and properties which we are 'deprecating' now, while we still remember =D

[Christian Dywan]

Created attachment 28553 [details]
Implement title and uri on the view #3

Updated patch, now with version 1.1.2 tags and without the GtkLauncher bit.

[Gustavo Noronha (kov)]

(In reply to comment #9)
> Created an attachment (id=28553) [review]
> Implement title and uri on the view #3
> 
> Updated patch, now with version 1.1.2 tags and without the GtkLauncher bit.
> 

Looks good to me. I'm in favor of r+'ing =).

[Adam Roben (:aroben)]

Comment on attachment 28553 [details]
Implement title and uri on the view #3

r=me

[Christian Dywan]

2009-03-16  Christian Dywan  <[e-mail]>

        Reviewed by Adam Roben.

        [gtk] API implementation: url and title
        http://bugs.webkit.org/show_bug.cgi?id=14807

        * WebCoreSupport/FrameLoaderClientGtk.cpp:
        (WebKit::FrameLoaderClient::dispatchDidChangeLocationWithinPage):
        (WebKit::FrameLoaderClient::dispatchDidReceiveTitle):
        (WebKit::FrameLoaderClient::dispatchDidCommitLoad):
        * webkit/webkitwebview.cpp:
        * webkit/webkitwebview.h: Implement "title" and "uri" properties as well
        as according functions. "uri" always reflects the current location
        including navigation inside the same page. title-changed is deprecated.

I adjusted the version tags while committing since we are targetting 1.1.4 now.