[Webkit-unassigned] [Bug 28975] Can't upload images with "Hide Extension" set to imgur.com on chromium/mac

[bugzilla-daemon at webkit.org]

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


Viet-Trung Luu <viettrungluu at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |viettrungluu at gmail.com




--- Comment #4 from Viet-Trung Luu <viettrungluu at gmail.com>  2009-09-04 16:01:50 PDT ---
IANAWKR, but some comments anyway:

> Index: WebCore/platform/chromium/FileChooserChromium.cpp

> +#if PLATFORM(DARWIN)
> +String pathGetPresentationalName(const String& path);
> +#endif

I don't think having an explicit prototype to something in another file is a
good idea; if it's that specific to this file, maybe the entire function
belongs in this file? However, see also my comments below.

Nit: I would prefer |pathGetDisplayFileName()|. (Why? I want the "File" in
there to indicate that it's just a display form of the file name, not the
entire path; "Display" is shorter and maybe also more standard than
"Presentational".)

> +#if PLATFORM(DARWIN)
> +        // See crbug.com/20857.
> +        string = pathGetPresentationalName(m_filenames[0]);
> +#else
>          string = pathGetFileName(m_filenames[0]);
> +#endif

I don't like this #ifdef; I'd rather see |pathGetPresentationalName()| just do
the same thing as |pathGetFileName()| on other platforms (again, see below).

> Index: WebCore/platform/chromium/FileSystemChromiumMac.mm

>  String pathGetFileName(const String& path)
[...]
> +String pathGetPresentationalName(const String& path)
[...]

This is fine in itself, but |pathGetFileName| has prototype in
WebCore/platform/FileSystem.h, so |pathGetPresentationalName()| (or whatever it
ends up being called) should logically be there too. I wouldn't be opposed to
that, even with an |#if (PLATFORM(CHROMIUM) && PLATFORM(DARWIN) [prototype ...]
#else [inline definition which makes pathGetPresentationName() equivalent to
pathGetFileName ...] #endif|. I can see that others may be opposed to this,
though.

Just my opinion -- as I said, IANAWKR.

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