========================================================================
http://mondrian.corp.google.com/file/9311198///depot/googleclient/gears/opensource/gears/desktop/desktop.cc?a=1
File //depot/googleclient/gears/opensource/gears/desktop/desktop.cc (snapshot 1)
------------------------------------
Line 1090: if (flavor_as_string == STRING16(L"Files")) {
On 12:49 pm, noel wrote:
> please use a case-insensitive string match here, see  
> base/common/string_utils.h StringCompareIgnoreCase()

Although IE is case-insensitive, the HTML5 spec says "the format strings are
opaque, case-sensitive, strings" [1]. WebKit is similarly case-sensitive [2].

[1] http://www.whatwg.org/specs/web-apps/current-work/multipage/editing.html#dnd

[2]
http://trac.webkit.org/browser/trunk/WebCore/platform/mac/ClipboardMac.mm#L64
========================================================================
http://mondrian.corp.google.com/file/9311198///depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_common.h?a=1
File 
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_common.h
 (snapshot 1)
------------------------------------
Line 45: DRAG_AND_DROP_FLAVOR_INVALID
On 12:38 pm, noel wrote:
> please make DRAG_AND_DROP_FLAVOR_INVALID the first entry of
> this enum, and give it a value, 0 would be good choice.  The compiler will
> auto-assign the remaining enum values so we don't need to worry about those.
> 
>    DRAG_AND_DROP_FLAVOR_INVALID = 0,
>    DRAG_AND_DROP_FLAVOR_FILES, 
>    DRAG_AND_DROP_FLAVOR_TEXT, 
>    DRAG_AND_DROP_FLAVOR_URL,
> 
> Consider doing the same for enum DragAndDropEventType while we are at it.

I prefer keeping the FOO_INVALID at the end of the enum, otherwise adding new
enum values means finicking around with whether or not a line needs to end with
a comma. With FOO_INVALID at the end, it's simple: everything gets a comma,
except for FOO_INVALID, which comes last.

What's the gain for making FOO_INVALID the first in the list (and hence zero)? I
prefer an explicit "if (foo != FOO_INVALID)" rather than saying "if (foo)" and
relying on FOO_INVALID also being zero. What if we, in the future, want to
distinguish between a FOO_INVALID and a FOO_UNKNOWN - which of those gets to be
the special zero value?
========================================================================

-- 
To respond, reply to this email or visit http://mondrian.corp.google.com/9311198

Reply via email to