Bug 17176 - [GTK] API: hovering-over-link and webkit_web_view_open /_load_foo
Summary: [GTK] API: hovering-over-link and webkit_web_view_open /_load_foo
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit API (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Linux
: P2 Major
Assignee: Christian Dywan
URL:
Keywords: Gtk
Depends on:
Blocks: 14141
  Show dependency treegraph
 
Reported: 2008-02-04 10:45 PST by Christian Dywan
Modified: 2009-02-28 12:14 PST (History)
4 users (show)

See Also:


Attachments
Readjust view and frame loading functions (9.85 KB, patch)
2009-01-24 21:33 PST, Christian Dywan
no flags Details | Formatted Diff | Diff
Readjust view and frame loading functions #2 (9.40 KB, patch)
2009-01-26 10:52 PST, Christian Dywan
no flags Details | Formatted Diff | Diff
Support file paths in _open for compatibility (1.02 KB, patch)
2009-02-26 12:28 PST, Christian Dywan
zecke: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Christian Dywan 2008-02-04 10:45:52 PST
The API of WebKitWebView has two remaining quirks.

1) If you want to load content into the view, you currently have the following functions:
  webkit_web_view_open
  webkit_web_view_load_string
  webkit_web_view_load_html_string
  webkit_web_frame_load_request

That is plain chaos right now. So my suggestion instead is the following:

  webkit_web_view_load_uri
  webkit_web_view_load_string
  webkit_web_view_load_request

  webkit_web_frame_load_uri
  webkit_web_frame_load_string
  webkit_web_frame_load_request

2) WebKitWebView has a signal "hovering-over-link" with the random signature (WebKitWebView*, const gchar* tooltip, const gchar* uri).

I suggest to remove this in favor of "element-motion". Now the uneasy question is what the signature is going to be. In the long term you will probably have a function looking like webkit_web_view_get_element_at_pos returning a DOM element. Right now we should just try to provide the very most common values, which are from my experience:

  link_uri
  image_uri
  is_control
  is_editable_control
  is_password_control

As for how to represent that in API I'm hesitating.

Suggestions, opinions, ideas welcome, especially from hackers that might have a use for something as fancy as custom context menu items.
Comment 1 Marco Barisione 2008-06-19 11:23:56 PDT
(In reply to comment #0)
> 1) If you want to load content into the view, you currently have the following
> functions:
>   webkit_web_view_open
>   webkit_web_view_load_string
>   webkit_web_view_load_html_string
>   webkit_web_frame_load_request
> 
> That is plain chaos right now. So my suggestion instead is the following:
> 
>   webkit_web_view_load_uri
>   webkit_web_view_load_string
>   webkit_web_view_load_request
> 
>   webkit_web_frame_load_uri
>   webkit_web_frame_load_string
>   webkit_web_frame_load_request

I completely agree on this. Alp: what do you think about this API change? If the new API seems ok I can make the required changes and submit a patch for review.

> 2) WebKitWebView has a signal "hovering-over-link" with the random signature
> (WebKitWebView*, const gchar* tooltip, const gchar* uri).
> 
> I suggest to remove this in favor of "element-motion". Now the uneasy question
> is what the signature is going to be. In the long term you will probably have a
> function looking like webkit_web_view_get_element_at_pos returning a DOM
> element. Right now we should just try to provide the very most common values,
> which are from my experience:
> 
>   link_uri
>   image_uri
>   is_control
>   is_editable_control
>   is_password_control
> 
> As for how to represent that in API I'm hesitating.

How about something similar to HitTestResult? At the beginning it could contain just the most important things like a get_link_uri() method, later we can add a method returning the DOM element for more advanced uses.
Comment 2 Christian Dywan 2009-01-24 21:33:04 PST
Created attachment 27005 [details]
Readjust view and frame loading functions

As suggested above, this patch deprecates:

  webkit_web_view_open
  webkit_web_view_load_html_string

and introduces:

  webkit_web_view_load_uri
  webkit_web_view_load_request

  webkit_web_frame_load_uri
  webkit_web_frame_load_string

For consistency I implemented all functions on the view using the according frame functions.
Comment 3 Holger Freyther 2009-01-26 05:26:27 PST
Could you post the thread init patch somewhere else? or rs=me on that and just commit it.
Comment 4 Christian Dywan 2009-01-26 10:52:58 PST
Created attachment 27039 [details]
Readjust view and frame loading functions #2

I want for the rubber stamp for the threading change. Updated patch to not include it anymore.
Comment 5 Christian Dywan 2009-02-12 11:47:09 PST
Comment on attachment 27039 [details]
Readjust view and frame loading functions #2

2009-02-12  Christian Dywan  <christian@twotoasts.de>

        Reviewed by Holger Freyther.

        http://bugs.webkit.org/show_bug.cgi?id=17176
        [GTK] API: hovering-over-link and webkit_web_view_open /_load_foo

        * webkit/webkitwebframe.cpp:
        * webkit/webkitwebframe.h:
        * webkit/webkitwebview.cpp:
        * webkit/webkitwebview.h: Introduce webkit_web_frame_load_uri,
        webkit_web_frame_load_string, webkit_web_view_load_uri and
        webkit_web_view_load_request and unify implementations.
Comment 6 Christian Dywan 2009-02-26 12:28:35 PST
Created attachment 28030 [details]
Support file paths in _open for compatibility

As discussed, webkit_web_view_open used to support local paths until very recently and some applications rely on this.

So this patch adds a test if the string is actually a local path. We are not changing webkit_web_view_load_uri because that function is new and we don't really encourage this kind of "guessing" on WebKit's side.

I tested it with devhelp, which doesn't show any content anymore due to this regression. Another example would be liferea.
Comment 7 Gustavo Noronha (kov) 2009-02-27 20:02:47 PST
(In reply to comment #6)
> Created an attachment (id=28030) [review]
> Support file paths in _open for compatibility

Looks good to me, and is release critical in my opinion - we need to get this in before we release.
Comment 8 Holger Freyther 2009-02-28 11:42:27 PST
Comment on attachment 28030 [details]
Support file paths in _open for compatibility

Okay, you might want to do undo my change to open the results of the webkit tests as well.
Comment 9 Christian Dywan 2009-02-28 12:14:15 PST
2009-02-28  Christian Dywan  <christian@twotoasts.de>

        Reviewed by Holger Freyther.

        * webkit/webkitwebview.cpp: Let webkit_web_view_open add file:// if a
        locale path is passed for compatibility, since we used to support that.