Summary: | [gtk] get|set encoding api | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Diego Escalante Urrelo <diegoe> | ||||||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED WONTFIX | ||||||||||
Severity: | Enhancement | CC: | alp, ap, christian, cosimoc, mrobinson, xan.lopez | ||||||||
Priority: | P2 | Keywords: | Gtk | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | OS X 10.5 | ||||||||||
Attachments: |
|
Description
Diego Escalante Urrelo
2008-02-27 20:23:59 PST
Created attachment 19427 [details]
A first draft
Go ahead and comment!
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. (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: Created attachment 27827 [details]
encoding.patch
This does what Mac and Win do as far as I can tell.
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. We need to add two things were overlooked initially: A property 'custom-encoding' A read-only function/ property 'encoding' that returns the default value Created attachment 28034 [details]
Add _get_encoding and properties
(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 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 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? 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. 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. (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. (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. WebKit1GTK+ has been removed. |