Bug 85286 - [GTK] Compilation warnings in RenderTheme
Summary: [GTK] Compilation warnings in RenderTheme
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Philippe Normand
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-01 09:14 PDT by Philippe Normand
Modified: 2012-05-03 07:03 PDT (History)
0 users

See Also:


Attachments
Patch (2.45 KB, patch)
2012-05-01 09:25 PDT, Philippe Normand
no flags Details | Formatted Diff | Diff
Patch (2.54 KB, patch)
2012-05-02 01:59 PDT, Philippe Normand
mrobinson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Normand 2012-05-01 09:14:18 PDT
../../Source/WebCore/platform/gtk/RenderThemeGtk.cpp: In member function ‘virtual WTF::String WebCore::RenderThemeGtk::fileListNameForWidth(const WebCore::FileList*, const WebCore::Font&, int, bool) const’:
../../Source/WebCore/platform/gtk/RenderThemeGtk.cpp:716:16: warning: unused variable ‘systemBasename’ [-Wunused-variable]
../../Source/WebCore/platform/gtk/RenderThemeGtk.cpp: At global scope:
../../Source/WebCore/platform/gtk/RenderThemeGtk.cpp:694:13: warning: ‘bool WebCore::stringByAdoptingFileSystemRepresentation(gchar*, WTF::String&)’ defined but not used [-Wunused-function]

Patch incoming.
Comment 1 Philippe Normand 2012-05-01 09:25:23 PDT
Created attachment 139639 [details]
Patch
Comment 2 Martin Robinson 2012-05-01 10:03:10 PDT
Comment on attachment 139639 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=139639&action=review

> Source/WebCore/platform/gtk/RenderThemeGtk.cpp:-717
> -    if (fileList->length() == 1) {
> -        CString systemFilename = fileSystemRepresentation(fileList->item(0)->path());
> -        gchar* systemBasename = g_path_get_basename(systemFilename.data());
> -    } else if (fileList->length() > 1)

I'm pretty sure the right thing to do is to actually use the basename of the file if there is only one of them. It looks like this code now returns fileButtonNoFilesSelectedLabel. Quite likely you need to add something like this here:

if (fileList->length())
    string = pathGetFileName(fileList->item(0)->path());
Comment 3 Philippe Normand 2012-05-02 01:59:31 PDT
Created attachment 139770 [details]
Patch
Comment 4 Martin Robinson 2012-05-02 08:14:20 PDT
Comment on attachment 139770 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=139770&action=review

> Source/WebCore/platform/gtk/RenderThemeGtk.cpp:707
>      String string = fileButtonNoFileSelectedLabel();
>      if (multipleFilesAllowed)
>          string = fileButtonNoFilesSelectedLabel();
>  
> -    if (fileList->length() == 1) {
> -        CString systemFilename = fileSystemRepresentation(fileList->item(0)->path());
> -        gchar* systemBasename = g_path_get_basename(systemFilename.data());
> -    } else if (fileList->length() > 1)
> -        return StringTruncator::rightTruncate(multipleFileUploadText(fileList->length()), width, font, StringTruncator::EnableRoundingHacks);
> +    if (fileList->length())
> +        string = pathGetFileName(fileList->item(0)->path());

Now that I look at this again, it seems like it should be structured as an if-else if-else block:

String string;
if (fileList->length())
    string = pathGetFileName(fileList->item(0)->path());
else if (multipleFilesAllowed)
    string = fileButtonNoFilesSelectedLabel();
else if
    string = fileButtonNoFileSelectedLabel();

Sorry for not mentioning it before.
Comment 5 Philippe Normand 2012-05-02 10:57:14 PDT
Committed r115852: <http://trac.webkit.org/changeset/115852>