Bug 17585 - [gtk] get|set encoding api
Summary: [gtk] get|set encoding api
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Enhancement
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2008-02-27 20:23 PST by Diego Escalante Urrelo
Modified: 2014-04-08 17:56 PDT (History)
6 users (show)

See Also:


Attachments
A first draft (1.77 KB, patch)
2008-02-27 20:25 PST, Diego Escalante Urrelo
no flags Details | Formatted Diff | Diff
encoding.patch (5.53 KB, patch)
2009-02-20 03:35 PST, Xan Lopez
no flags Details | Formatted Diff | Diff
Add _get_encoding and properties (5.11 KB, patch)
2009-02-26 13:07 PST, Christian Dywan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Diego Escalante Urrelo 2008-02-27 20:23:59 PST
Attached patch implementes a get|set for encoding in webview, it's half-working because it seems I'm doing something silly with the string returned.
Attached to the patch is the epiphany part consuming this API, it's there for reference.

What ephy needs is:
- a way to get the current encoding for the page so the view > encoding menu can be displayed correctly (the proper current encoding selected)
- a way to tell the current web view to override the default-encoding of websettings for the current website (and reload the page when the encoding is changed).

The patch provides the two first thing (with bugs), the second thing should be a matter of adding a reload() somewhere in the code, right now setting the encoding and manually reloading changes nothing, I guess it's in part fault of my buggy implementation.
Please comment so I can learn the ways of C++ :)
Comment 1 Diego Escalante Urrelo 2008-02-27 20:25:01 PST
Created attachment 19427 [details]
A first draft

Go ahead and comment!
Comment 2 Alp Toker 2008-04-06 20:04:18 PDT
We usually avoid g_strdup() for returned strings in favour of const gchar* and caching the value for API consistency.

I'll try to look over this for correctness soon (incl. comparing with how the other ports do this) unless someone beats me to it. Thanks for the patch.
Comment 3 Alp Toker 2008-08-10 11:08:50 PDT
(In reply to comment #1)
> Created an attachment (id=19427) [edit]
> A first draft
> 
> Go ahead and comment!
> 

Does this change the encoding of the currently loaded document?

Mac WebView does it a little differently:

http://developer.apple.com/documentation/Cocoa/Reference/WebKit/Classes/WebView_Class/Reference/Reference.html#//apple_ref/occ/instm/WebView/setCustomTextEncodingName:
Comment 4 Xan Lopez 2009-02-20 03:35:38 PST
Created attachment 27827 [details]
encoding.patch

This does what Mac and Win do as far as I can tell.
Comment 5 Holger Freyther 2009-02-26 07:47:23 PST
Comment on attachment 27827 [details]
encoding.patch


> +    if (!overrideEncoding.isEmpty()) {
> +      WebKitWebViewPrivate* priv = webView->priv;
> +      g_free (priv->customEncoding);
> +      priv->customEncoding = g_strdup(overrideEncoding.utf8().data());
> +      return priv->customEncoding;
> +    } else
> +      return NULL;
> +}

looks like two style issues here. "g_free (" and two instead of four spaces to indent... please fix when landing.
Comment 6 Holger Freyther 2009-02-26 08:22:06 PST
Landed in r41247.
Comment 7 Christian Dywan 2009-02-26 12:43:44 PST
We need to add two things were overlooked initially:

A property 'custom-encoding'

A read-only function/ property 'encoding' that returns the default value
Comment 8 Christian Dywan 2009-02-26 13:07:40 PST
Created attachment 28034 [details]
Add _get_encoding and properties
Comment 9 Christian Dywan 2009-02-27 14:01:28 PST
(In reply to comment #8)
> Created an attachment (id=28034) [review]
> Add _get_encoding and properties

Meanwhile I implemented menu items for this to properly test the patch. _get_encoding seems to return the custom encoding, ie. loader()->encoding() actually has the custom value after the custom encoding set for the first time. I'm not sure yet how to overcome this.
Comment 10 Gustavo Noronha (kov) 2009-02-28 06:00:15 PST
Comment on attachment 27827 [details]
encoding.patch

Clearing the review flag so that it stops showing up as a pending commit 'till we got the other one reviewed.
Comment 11 Holger Freyther 2009-03-01 07:47:52 PST
Comment on attachment 28034 [details]
Add _get_encoding and properties


> +    /**
> +    * WebKitWebView:custom-encoding:
> +    *
> +    * The custom encoding of the web view.
> +    *
> +    * Since: 1.1.1
> +    */

may I get another space here?
Comment 12 Christian Dywan 2009-03-01 11:01:35 PST
2009-03-01  Christian Dywan  <christian@twotoasts.de>
    
            Reviewed by Holger Freyther.
    
            * webkit/webkitprivate.h:
            * webkit/webkitwebview.cpp:
            (webkit_web_view_get_encoding):
            * webkit/webkitwebview.h: Implement 'encoding' and 'custom-encoding'
            properties as well as webkit_web_view_get_encoding.
Comment 13 Christian Dywan 2009-03-01 11:02:57 PST
Note that I changed the Since tags of the properties to 0.1.2, but I'm unsure whether that's the right version. We should discuss that. Leaving the bug open for now.
Comment 14 Xan Lopez 2009-03-01 13:48:26 PST
(In reply to comment #9)
> (In reply to comment #8)
> > Created an attachment (id=28034) [review] [review]
> > Add _get_encoding and properties
> 
> Meanwhile I implemented menu items for this to properly test the patch.
> _get_encoding seems to return the custom encoding, ie. loader()->encoding()
> actually has the custom value after the custom encoding set for the first time.
> I'm not sure yet how to overcome this.
> 

If this is really the case it might make sense to just have one function, get_encoding, and another one to check whether or not the encoding is automatic or forced by the user. Not much sense in having two getters that return the same thing, I think.
Comment 15 Christian Dywan 2009-03-01 16:02:30 PST
(In reply to comment #14)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > Created an attachment (id=28034) [review] [review] [review]
> > > Add _get_encoding and properties
> > 
> > Meanwhile I implemented menu items for this to properly test the patch.
> > _get_encoding seems to return the custom encoding, ie. loader()->encoding()
> > actually has the custom value after the custom encoding set for the first time.
> > I'm not sure yet how to overcome this.
> > 
> 
> If this is really the case it might make sense to just have one function,
> get_encoding, and another one to check whether or not the encoding is automatic
> or forced by the user. Not much sense in having two getters that return the
> same thing, I think.

Well, it's not expected to make any sense, it's a bug afterall. As we discussed, the API should allow for seeing the default encoding even if it was overridden.

To me it looks like a bug in WebCore at this point, but I have to give that a closer look since I'm not familiar with where that's coming from. Otherwise we need to workaround it in WebKit I guess.
Comment 16 Martin Robinson 2014-04-08 17:56:24 PDT
WebKit1GTK+ has been removed.