WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
47990
[GTK] Replace encodings multi-dimensional array with calls to registrar method
https://bugs.webkit.org/show_bug.cgi?id=47990
Summary
[GTK] Replace encodings multi-dimensional array with calls to registrar method
Carlos Garcia Campos
Reported
2010-10-20 09:38:49 PDT
The complicated multidimensional array used for encodings and aliases in TextCodecGtk could be replaced by calls to registrar. It would also fix the coding style since current variable names contain underscores.
Attachments
Patch
(31.27 KB, patch)
2010-10-20 09:42 PDT
,
Carlos Garcia Campos
mrobinson
: review-
Details
Formatted Diff
Diff
Updated patch
(32.93 KB, patch)
2010-10-21 00:49 PDT
,
Carlos Garcia Campos
mrobinson
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Carlos Garcia Campos
Comment 1
2010-10-20 09:42:00 PDT
Created
attachment 71300
[details]
Patch
Martin Robinson
Comment 2
2010-10-20 12:28:11 PDT
Comment on
attachment 71300
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=71300&action=review
I love this patch. Just a couple small nits.
> WebCore/platform/text/gtk/TextCodecGtk.cpp:92 > +bool TextCodecGtk::registerEncodingAliasIfAvailable(EncodingNameRegistrar registrar, const char* canonicalName, const char* aliasName)
It looks like you never check the return value of this method. Can it just return void?
> WebCore/platform/text/gtk/TextCodecGtk.h:60 > + static bool registerEncodingNameIfAvailable(EncodingNameRegistrar registrar, const char* canonicalName); > + static bool registerEncodingAliasIfAvailable(EncodingNameRegistrar registrar, const char* canonicalName, const char* aliasName); > + static void registerCodecIfAvailable(TextCodecRegistrar registrar, const char* codecName); >
Could these these can just be static functions in TextCodecGtk.cpp?
Carlos Garcia Campos
Comment 3
2010-10-21 00:49:48 PDT
Created
attachment 71400
[details]
Updated patch
Martin Robinson
Comment 4
2010-10-21 00:57:36 PDT
Comment on
attachment 71400
[details]
Updated patch View in context:
https://bugs.webkit.org/attachment.cgi?id=71400&action=review
Great! Thank you.
> WebCore/platform/text/gtk/TextCodecGtk.cpp:62 > +static bool isEncodingAvailable(const gchar* encName)
encName should be replaced with encodingName, since we are avoiding abbreviations in new code. I'll fix this and land it myself. :)
Martin Robinson
Comment 5
2010-10-21 01:08:40 PDT
Committed
r70211
: <
http://trac.webkit.org/changeset/70211
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug