[webkit-reviews] review denied: [Bug 50738] [GTK] Choose a dictionary in case there is no one defined or the env LANG does not propose one : [Attachment 76025] Proposed patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has denied Alejandro G. Castro
<alex at igalia.com>'s request for review:
Bug 50738: [GTK] Choose a dictionary in case there is no one defined or the env
LANG does not propose one
https://bugs.webkit.org/show_bug.cgi?id=50738

Attachment 76025: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=76025&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=76025&action=review

Yay! Thanks for fixing this.

> WebKit/gtk/webkit/webkitwebsettings.cpp:953
> +static void get_avail_dicts_cb(const char * const lang_tag, const char *
const provider_name, const char * const provider_desc, const char * const
provider_file, void * data)

Should be in WebKit style (camelCase), asterisks with the typenames and use
full words instead of abbreviations. :)

> WebKit/gtk/webkit/webkitwebsettings.cpp:1079
> +		   // No dictionaries selected, we get one from the list
> +		   enchant_broker_list_dicts(broker, get_avail_dicts_cb,
&dicts);
> +		   if (dicts) {
> +		       GSList* tmp;

Instead of passing a GSList* it might simplify things to pass a point to a
Vector<CString> like:

Vector<CString> allDictionaries;
enchant_broker_list_dicts(broker, getAvilableDictionariesCallback,
&allDictionaries);
if (!allDictionaries.empty()) {
      ...
     
}