[Webkit-unassigned] [Bug 68996] [GTK][WEBKIT2] Add Font and Encoding properties to WebKitWebSettings.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=68996





--- Comment #3 from Nayan Kumar K <nayankk at motorola.com>  2011-09-28 08:36:46 PST ---
(From update of attachment 109007)
View in context: https://bugs.webkit.org/attachment.cgi?id=109007&action=review

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:1143
>> +
> 
> it should 0 instead of FALSE

Done

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:1146
>> +    const gchar* defaultFontFamily = WKStringGetCString(standardFontFamilyRef);
> 
> This is wrong, WKStringGetCString() doesn't return a const char * but a new allocated string, so you are leaking it. I like the idea of public methods returning a const char *, but for that we need to cache the uft8 string and return it.

New approach caches the string and deletes in upon WebKitWebSetting unref. It also updates the cached string upon calling *_set_font_family functions.

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:1170
>> +    g_object_notify(G_OBJECT(settings), "default-font-family");
> 
> You should check whether the property actually changed before emitting notify.

Done

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:1186
>> +
> 
> 0 instead of FALSE

Done

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:1189
>> +    const gchar* monospaceFontFamily = WKStringGetCString(fixedFontFamilyRef);
> 
> Same leak here.

Done

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:1213
>> +    g_object_notify(G_OBJECT(settings), "monospace-font-family");
> 
> Check it changed before emitting notify

Done

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:1229
>> +
> 
> 0 instead of FALSE

Done

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:1232
>> +    const gchar* serifFontFamily = WKStringGetCString(serifFontFamilyRef);
> 
> Same leak here.

Done.

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:1256
>> +    g_object_notify(G_OBJECT(settings), "serif-font-family");
> 
> Check it changed before emitting notify.

Done.

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:1272
>> +
> 
> 0 instead of FALSE

Done

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:1275
>> +    const gchar* sansSerifFontFamily = WKStringGetCString(sansSerifFontFamilyRef);
> 
> Same leak here.

Done.

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:1299
>> +    g_object_notify(G_OBJECT(settings), "sans-serif-font-family");
> 
> Check it changed before emitting notify.

Done.

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:1315
>> +
> 
> 0 instead of FALSE.

Done

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:1318
>> +    const gchar* cursiveFontFamily = WKStringGetCString(cursiveFontFamilyRef);
> 
> Same leak here.

Done.

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:1342
>> +    g_object_notify(G_OBJECT(settings), "cursive-font-family");
> 
> Check it changed before emitting notify.

Done.

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:1358
>> +
> 
> 0 instead of FALSE.

Done

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:1361
>> +    const gchar* fantasyFontFamily = WKStringGetCString(fantasyFontFamilyRef);
> 
> Same leak here.

Done.

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:1385
>> +    g_object_notify(G_OBJECT(settings), "fantasy-font-family");
> 
> Check it changed before emitting notify.

Done.

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:1401
>> +
> 
> 0 instead of FALSE.

Done

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:1404
>> +    const gchar* pictographFontFamily = WKStringGetCString(pictographFontFamilyRef);
> 
> Same leak here.

Done

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:1428
>> +    g_object_notify(G_OBJECT(settings), "pictograph-font-family");
> 
> Check it changed before emitting notify.

Done

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:1444
>> +
> 
> 0 instead of FALSE

Done

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:1485
>> +
> 
> 0 instead of FALSE.

Done

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:1526
>> +
> 
> 0 instead of FALSE.

Done

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:1567
>> +
> 
> 0 instead of FALSE.

Done

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:1570
>> +    const gchar* defaultEncoding = WKStringGetCString(defaultEncodingRef);
> 
> Same leak here.

Done.

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:1594
>> +    g_object_notify(G_OBJECT(settings), "default-encoding");
> 
> Check it changed before emitting notify.

Done.

>> Source/WebKit2/UIProcess/API/gtk/tests/testwebsettings.c:150
>> +    g_assert(!g_strcmp0(webkit_web_settings_get_default_font_family(settings), "sans-serif"));
> 
> Use g_assert_cmpstr().

Quite useful macro! Done.

>> Source/WebKit2/UIProcess/API/gtk/tests/testwebsettings.c:152
>> +    g_assert(!g_strcmp0(webkit_web_settings_get_default_font_family(settings), "monospace"));
> 
> Ditto.

Done

>> Source/WebKit2/UIProcess/API/gtk/tests/testwebsettings.c:160
>> +    g_assert(!g_strcmp0(webkit_web_settings_get_monospace_font_family(settings), "monospace"));
> 
> Ditto.

Done.

>> Source/WebKit2/UIProcess/API/gtk/tests/testwebsettings.c:162
>> +    g_assert(!g_strcmp0(webkit_web_settings_get_monospace_font_family(settings), "sans-serif"));
> 
> Ditto.

Done.

>> Source/WebKit2/UIProcess/API/gtk/tests/testwebsettings.c:171
>> +    g_assert(!g_strcmp0(webkit_web_settings_get_serif_font_family(settings), "serif"));
> 
> Ditto.

Done.

>> Source/WebKit2/UIProcess/API/gtk/tests/testwebsettings.c:173
>> +    g_assert(!g_strcmp0(webkit_web_settings_get_serif_font_family(settings), "sans-serif"));
> 
> Ditto.

Done.

>> Source/WebKit2/UIProcess/API/gtk/tests/testwebsettings.c:182
>> +    g_assert(!g_strcmp0(webkit_web_settings_get_sans_serif_font_family(settings), "sans-serif"));
> 
> Ditto.

Done.

>> Source/WebKit2/UIProcess/API/gtk/tests/testwebsettings.c:184
>> +    g_assert(!g_strcmp0(webkit_web_settings_get_sans_serif_font_family(settings), "serif"));
> 
> Ditto.

Done.

>> Source/WebKit2/UIProcess/API/gtk/tests/testwebsettings.c:195
>> +    g_assert(!g_strcmp0(webkit_web_settings_get_cursive_font_family(settings), "sans-serif"));
> 
> Ditto.

Done.

>> Source/WebKit2/UIProcess/API/gtk/tests/testwebsettings.c:204
>> +    g_assert(!g_strcmp0(webkit_web_settings_get_fantasy_font_family(settings), "serif"));
> 
> Ditto.

Done.

>> Source/WebKit2/UIProcess/API/gtk/tests/testwebsettings.c:206
>> +    g_assert(!g_strcmp0(webkit_web_settings_get_fantasy_font_family(settings), "sans-serif"));
> 
> Ditto.

Done.

>> Source/WebKit2/UIProcess/API/gtk/tests/testwebsettings.c:215
>> +    g_assert(!g_strcmp0(webkit_web_settings_get_pictograph_font_family(settings), "serif"));
> 
> Ditto.

Done.

>> Source/WebKit2/UIProcess/API/gtk/tests/testwebsettings.c:217
>> +    g_assert(!g_strcmp0(webkit_web_settings_get_pictograph_font_family(settings), "sans-serif"));
> 
> Ditto.

Done.

>> Source/WebKit2/UIProcess/API/gtk/tests/testwebsettings.c:225
>> +    g_assert(webkit_web_settings_get_default_font_size(settings) == 12);
> 
> Use g_assert_cmpuint().

Done.

>> Source/WebKit2/UIProcess/API/gtk/tests/testwebsettings.c:227
>> +    g_assert(webkit_web_settings_get_default_font_size(settings) == 14);
> 
> Ditto.

Done.

>> Source/WebKit2/UIProcess/API/gtk/tests/testwebsettings.c:236
>> +    g_assert(webkit_web_settings_get_default_monospace_font_size(settings) == 10);
> 
> Ditto.

Done.

>> Source/WebKit2/UIProcess/API/gtk/tests/testwebsettings.c:238
>> +    g_assert(webkit_web_settings_get_default_monospace_font_size(settings) == 12);
> 
> Ditto.

Done.

>> Source/WebKit2/UIProcess/API/gtk/tests/testwebsettings.c:247
>> +    g_assert(webkit_web_settings_get_minimum_font_size(settings) == 5);
> 
> Ditto.

Done.

>> Source/WebKit2/UIProcess/API/gtk/tests/testwebsettings.c:249
>> +    g_assert(webkit_web_settings_get_minimum_font_size(settings) == 7);
> 
> Ditto.

Done.

>> Source/WebKit2/UIProcess/API/gtk/tests/testwebsettings.c:257
>> +    g_assert(!g_strcmp0(webkit_web_settings_get_default_encoding(settings), "iso-8859-1"));
> 
> Use g_assert_cmpstr().

Done.

>> Source/WebKit2/UIProcess/API/gtk/tests/testwebsettings.c:259
>> +    g_assert(!g_strcmp0(webkit_web_settings_get_default_encoding(settings), "utf8"));
> 
> Ditto.

Done.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.