This is a really important addition to the browser and I'm super excited to have this added. That said, it's a lot of code and so there are a lot of tiny issues which could be addressed:

Icons - it would be nice to use stock items/named icons (providing a fallback image) so that themes and users can override and theme the icons. This could be done after merging the extension.



main.c:38: the "/* Preferences of this extension should be opened */" comment above _nojs_on_preferences_response seems to go with the next function.



nojs.c:96: it looks like _nojs_closure_VOID__STRING_ENUM is a generated function by glib-genmarshal. It would be nice to do this at build-time rather than embedding generated code directly, so it's easier to change later. This isn't a critical change for now.

nojs.c:177: it seems this would leak priv->databaseFilename.

nojs.c:209: typo, "extenstion"

I'm not really qualified to comment on the SQLite calls, but nothing sticks out as wrong. However, I'm curious about nojs.c:255 and nojs.c:270. Is line 268 guaranteed to set error to NULL if no error occurs? If not, line 270 could be a double-free. It's probably clearer to set error to NULL by hand anyway after freeing the error on line 255.

nojs.c:786: I've run into this webkit behavior as well, and it really sucks. Your workaround seems as good as any. :)

nojs.c:905: I don't think soup_uri_get_host will return "", but if it does, this will read from domain[-1]. In any event, the loop here would be more clear if you used strrchr instead. Regardless, a comment to say exactly what all the looping and char-comparisons are intended to do would be good.

nojs.c:1035: typo, "domanins"



nojs-preferences.c:95: this should probably be a const gchar*, since the function neither owns its memory nor modifies its contents; this better matches the return type of sqlite3_column_text.

nojs-preferences.c:161: "extenstion" again

nojs-preferences.c:177: no need for explicit return.

nojs-preferences.c:185: typo, "toogle". Same on 207 and 223, 245, 261, 281, 

nojs-preferences.c:326: it seems like doing one-item delete queries in a for loop is probably not the most efficient way to interface with the db here, but for now this is fine. An XXX or TODO comment might be worth adding.

nojs-preferences.c:373: "cookie" should be "script" or "JS". This is duplicated in a variety of places in the file.

nojs-preferences.c:607: "#ifdef GTK__3_0_VERSION" should be "#if GTK_CHECK_VERSION(3, 0, 0)". Same for other references to "GTK__3_0_VERSION".

nojs-preferences.c:630: g_strdup_printf is called with a format-string argument derived from translations without any arguments to format. The strdup/free pair can be completely removed here.



nojs-view.c:93: it might be better to use G_STRFUNC here instead of __func__. I'm not sure of the exact reason, but I would expect the former is safer w/r/t portability. Maybe this print should be hidden behind a debugging environment variable as well. Not a blocking concern.

nojs-view.c:292: typos, "seperator" and "seperate"

nojs-view.c:548: typo, "ourselve"



Throughout the code: Generally we don't require extensions to conform to the Midori coding style as long as there's internal consistency. With respect to internal consistency, I don't follow the logic behind indentation of continued lines like the following:
G_DEFINE_TYPE(NoJS,
				nojs,
				G_TYPE_OBJECT)

I couldn't find a tab-width which caused this to align exactly with the opening-paren, which is also the case for a variety of places in the code. Is the rule to use enough tabs that a 4-width tab width brings continued lines at least to the location after opening braces/parentheses?

(Personally I would prefer just a single tab for continued lines, or using spaces to indent and spaces to align. But the policy of not imposing coding style too much on extensions remains. ;))


Thanks for all the work, this code looks very solid overall!