Summary: | [GTK] Clean up WebCore/platform/graphics/gtk/ImageGtk.cpp | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Martin Robinson <mrobinson> | ||||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, eric, gustavo, webkit.review.bot, xan.lopez | ||||||
Priority: | P3 | Keywords: | Gtk | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | PC | ||||||||
OS: | Linux | ||||||||
Attachments: |
|
Description
Martin Robinson
2010-08-16 13:07:53 PDT
Created attachment 64516 [details]
Cleanup patch
Attachment 64516 [details] did not build on gtk: Build output: http://queues.webkit.org/results/3756311 Created attachment 64530 [details]
Patch with build fixes
Comment on attachment 64530 [details] Patch with build fixes WebCore/platform/graphics/gtk/ImageGtk.cpp:58 + if (!GetModuleFileName(hmodule, (CHAR *) dataDirectory, sizeof(retval) - 10)) Should this (CHAR *) be turned into a static_cast<CHAR*>(dataDirectory) for greater style guide followage? WebCore/platform/graphics/gtk/ImageGtk.cpp:141 + GOwnPtr<gchar> glibFileName(g_build_filename(getWebKitDataDirectory(), "webkit-1.0", "images", imageName.get(), NULL)); Data directories should follow the "soname" of the library. We have done it in InspectoClientGtk: http://trac.webkit.org/browser/trunk/WebKit/gtk/WebCoreSupport/InspectorClientGtk.cpp#L90 Can you then also please change WebCore/GNUmakefile.am to add 'gtk' after 'webkit' here? You'll notice the inspector one is already correct, but we missed images =(: http://trac.webkit.org/browser/trunk/WebCore/GNUmakefile.am#L4575 As we discussed on IRC, it would be great to move the getWebKitDataDirectory function to, say, FileSystemGtk, and then use it in InspectorClient as well, so it starts actually working on Windows heh. Let's follow up on that later! Committed r65529: <http://trac.webkit.org/changeset/65529> http://trac.webkit.org/changeset/65529 might have broken GTK Linux 64-bit Debug The following changes are on the blame list: http://trac.webkit.org/changeset/65529 http://trac.webkit.org/changeset/65530 |